[RFC] Adding initial SVE implementation

Introducing the addition of Arm Architecture's Scalable Vector Extension (SVE) in TVM containing initial VLA and predication implementation, based on earlier work by Giuseppe Rossini.
@mbaret

I think it is a good idea to invite people working on RISC-V support for TVM, since the RISC V vector extension is similar to ARM SVE. I remember some people in Taiwan are working on this. Maybe @comaniac knows who?

I think it is a good idea to invite people working on RISC-V support for TVM for review/discuss, since the RISC V vector extension is similar to ARM SVE. I remember some people in Taiwan are working on this. Maybe @comaniac knows who?

Thanks for bringing up. cc @yrchen

@sjoerdmeijer @giuseros

Thanks @MeeraN7 for the RFC. SVE is certainly an interesting topic that can be covered in different ways.
It would be very helpful to also show examples of the code along the transformations, so we can have a better understanding the possible design tradeoffs.
Specifically:

The TIR before SVE vectorization
The TIR after SVE vectorization right before LLVM codegen
The corresponding lowered llvm intrinsics

cc @junrushao1994 @vinx13 as it can be related to the future tensorization optimizations

HI @tqchen ,
I will try to sporadically comment, since this is a project I prototyped (and enjoyed :) ) when I was in Arm.
If I understand your comment correctly, what @MeeraN7 is doing is closer to what you are proposing. Instead of transforming a loop into a Ramp, and passing the ramp "as is" to LLVM (which is what is done for fixed vector length, but not doable for SVE) @MeeraN7 is legalising the loop in TIR and passing the legal loop down to the LLVM code-generator. In other words, the following loop:
for i = 1:1:10
A[i] = B[i]+C[i];
end

Once legalised becomes
for i = 1:VL:10
A[VL] = B[VL]+C[VL]
end

And then the LLVM code generator, knowing that this is a variable loop, translates this it with some LLVM intrinsics for:

Predicated load/store
Loop increment
Predication mask calculation
Please note that only load/store needs to be predicated. Other register-to-register operations (e.g., add/sub/mul)  won't need predication.

Please also note that while predication is not a TIR concept, we use it to support VLA (cc @tkonolige) in the LLVM codegen. In future it should be quite straightforward to expose predication also in TIR (if required).
@MeeraN7 feel free to jump in if something I said is not correct (very likely :) )

Thank you @giuseros for your answer, it looked good to me. Just to add on, we know which loops need to be legalised as in TIR we do add a new ForKind called kVectorizedScalable (cc @tkonolige) which marks the loop as able to be vectorized but the value of VL is unknown. These loops are then legalised and transformed to include the unknown value VL and later passed to the code generator as @giuseros mentioned.

Thank you all for the comments so far, we are currently working on a response to address them.

Thanks @MeeraN7 . I think the overall loop legalization makes sense. I wonder then if it is necessary to update the node construct such as Ramp, or can we directly make use of scalar loop in the body

@masahi, about:

If possible, I think it is better not to introduce user facing changes, since as far as an API is concerned, s[C].vectorize(...) is already vector-length agnostic.

which I think is very closely related to an earlier inline comment:

As I commented above, I'd like to continue using s[C].vectorize(...) and when the feature is available, enable SVE by a target attribute. So I don't expect any user facing work.

I think we do need a user-facing option to toggle fixed/scalable vectorisation. If the vectorisation strategy is selected based on an available target attribute, we loose control to choose fixed/scalable vectorisation. For architectures that do support scalable vectorisation, fixed width might still be preferable in some cases.
I think this is similar to Clang's loop pragmas. For example, the vectorise_width pragma has been extended with an optional second argument fixed|scalable:
vectorize_width(_value_[, fixed|scalable]),
see also the Clang docs here.
So I see two approaches:

we extend s[C].vectorize(...) to take an optional fixed/scalable boolean value, similar to Clang's loop pragma, which defaults to fixed if omitted,
or introduce s[C].vectorize_scalable(...) as proposed in this RFC.

I personally don't have any preference. But now I am wondering if extending s[C].vectorize(...), the first option, would be better. What do you think?

Thanks @sjoerdmeijer @giuseros, I didn't imagine that there would be a case where mixing fixed and scalable vectorization is beneficial. I prefer s[C].vectorize(..., scalable=True) to s[C].vectorize_scalable(...) but both seem fine.
Any other comments @tqchen @tkonolige?

Thanks @MeeraN7 @giuseros, to make the discussion more concrete, right now the IR after legalization looks like
  for (i: int32, 0, 17;i+=VL) {
    C_2[ramp(i, 1, VL)] = ((int8xVL*)A_2[ramp(i, 1, VL)] + (int8xVL*)B_2[ramp(i, 1, VL)])
  }

This would require changes such as the ramp data structure and data type to support the VL vector types. I wonder if they are necessary.  Here is a possible alternative way to do so
  for (i: int32, 0, 17;i, annotation={"VLA"}) {
    C_2[i] = A_2[i] + B_2[i];
  }

And we will be defering the vectorized instruction generation to the codegen phase, by specially handling the patterns in the for that is annotated with VLA loop. Of course we can only support a limited set of patterns(such as read/write to the same vector index or limited reduction support), that is why legalize is needed to make sure the body of VLA for loop satiesfies the pattern.
In this way we can likely get a similar set of things without hacking into get a ramp with VL size

Hi @tqchen, thank you for the comment. To clarify a few things, VL is not added to the Ramp Node at all, it is simply a string that is used when printing TIR for visual representation. The only addition to the Ramp Node (and also Broadcast Node) is a boolean called "is_scalable" which should not affect anything else as a separate constructor was added. I don't think any other information needs to be added to these nodes or data types.

Thanks @MeeraN7 . Yes I get what you mean. Right now we are adding a "is_scalable" field to indicate that the broadcast and ramp are "context dependent" on VL. Additionally, we might need to update DataType to indicate a scalable data type.
This context dependency is the missing information I mentioned here. The set of code is really undefined and should be parameterized by VL.  Additionally, considering the case of two loops with different VL1 and VL2  and  want to do some transformations, we might fall into the trap of thinking them as same type(because only "is_scalable" is marked) but in reality they are not, as a implicit dependency on VL can be ignored.
I can understand that the additional flag can be helpful as we could reuse some of the vectorization logic. However, the "is_scalable" field might introduce additional confusion as above, and the additional ramp node may not carry too much additional information(apart from the fact that we use a scalar vs a vector type). So my main question is that whether or not we could use a separate normal form to hint the code generator without changing the current DataType, ramp and broadcast.
Specifically, a regular loop as follows would carry same amount of information(the access to i(VLA index)) would indicate a vector load,  and access to other indices would become a normal load. The main constraint could be that we can only allow i to appear in certain locations(say in the inner most to represent a ramp like pattern), and defer the generation of SVE code to the codegen phase, by pattern matching the indices and lookup intermediate value:
N1: A possible loop normal form via annotation
  for (i: int32, 0, 17;i, annotation={"VLA"}) {
    C_2[i] = A_2[i] + B_2[i];
  }

N1 would indeed push a bit more pressure to the code generator, because now code generator needs to pattern match load/store of VLA index(i), and possibly perform broadcast if necessary. However, the additional overhead may not be too large and could help us to keep the code cleaner.  My guess is that this approach is also easier to generalize to later loop patterns such as scalable matrix instruction, in which case we cannot really reuse ramp and broadcast

Thanks for commenting @tqchen
Could you further clarify a few things for me please? See remarks inlined.

Thanks @MeeraN7 . Yes I get what you mean. Right now we are adding a "is_scalable" field to indicate that the broadcast and ramp are "context dependent" on VL. Additionally, we might need to update DataType to indicate a scalable data type.
This context dependency is the missing information I mentioned here.

I don't think I understand what you mean by context dependency. Basically, in my view of the world, the Ramp node means that we can process elements in a data parallel way. How exactly this is done, is up to backends depending on the architecture and the code. What we are doing here is annotating this Ramp node with a bit of state, which is a hint that we want a special kind of vectorisation. From this point of view it is syntactic sugar: if the hint is dropped or ignored, it could still be vectorised, but not in a vector length agnostic way.
I don't think we need to encode much more information than one boolean that specifies the vectorisation style. You could indeed argue that this is ad-hoc, but if we are going to do it in a different way, we would still need to keep a bit of state around, like the annotation={"VLA"} example that you gave earlier. From that point of view, I don't see any difference.

The set of code is really undefined and should be parameterized by VL.

What do you mean by undefined here?

Additionally, considering the case of two loops with different VL1 and VL2 and want to do some transformations, we might fall into the trap of thinking them as same type(because only "is_scalable" is marked) but in reality they are not, as a implicit dependency on VL can be ignored.
I can understand that the additional flag can be helpful as we could reuse some of the vectorization logic. However, the "is_scalable" field might introduce additional confusion as above, and the additional ramp node may not carry too much additional information(apart from the fact that we use a scalar vs a vector type). So my main question is that whether or not we could use a separate normal form to hint the code generator without changing the current DataType, ramp and broadcast.

Correct me if I am wrong, but your assumption is that explicit scalable state is bad. Looking at your example:

N1: A possible loop normal form via annotation
  for (i: int32, 0, 17;i, annotation={"VLA"}) {
    C_2[i] = A_2[i] + B_2[i];
  }

This annotation looks equivalent to a loop pragma. In Clang you can for example do:
 #pragma loop vectorize_width(4, scalable)
  for (I =0; I<17; ++i) {
    C_2[i] = A_2[i] + B_2[I];
  }

and thus request scalable vectorisation. If this annotation results in scalable vectorisation, the LLVM vectoriser will lower this to operations using <n x 4 x i32> scalable vector IR types.
What I would like to say with these examples, is that there are 2 things going on at different levels. I think:

The annotation={"VLA"} corresponds to a loop pragma in Clang,
The TIR Ramp node extension corresponds to LLVM IR scalable types, e.g. <n x 4 x i32>.

And I think these 2 concepts are different things that both have their value and place.
I we can only annotate loops as being scalable, we loose the finer grained control to request this on a statement level. I don't know if mixing fixed and scalable will be an important use-case, but I think it is possible.
Summarising, I don't think explicit encoding of scalable in TIR nodes is a bad thing, the opposite actually, I think we need it, and the annotation on the loop might be a complementary technique to this.
What do you think?

Thanks @sjoerdmeijer  , sorry for getting back to this late. If LLVM also encodes the SVE vector as a special type(and not tying the n) it could be a precedence that we can learn from. I do want to point out that the same approach won't work when it comes to scalable matrix instruction, so it would be good to think about a loop pattern based approach from that angle.
If we conclude that we really want to introduce the scalable vector into data type and so on. A remaining thing we need to resolve is the encoding. DataType and DLDataType are part of DLPack standard, used as standard ABI convention. So ideally we do not want to change the type signature of DataType DLDataType. What we might be able to is to introduce a magic lane number (say DataType::kScalableVectorLaneMark = -1) that indicate a scalable vector type, and also use similar approaches for other language construct as well. This would alleviate the problems from that side.
This being said. It would still be good to get @MeeraN7 @giuseros @sjoerdmeijer 's take on the possibility of using a normalized loop pattern vs embedding into the vector.
Thank you for all the discussions so far!

Hi @tqchen , thanks for the great feedback!

This being said. It would still be good to get @MeeraN7 @giuseros @sjoerdmeijer 's take on the possibility of using a normalized loop pattern vs embedding into the vector.

Yes, I can see that as being very useful. If my analogy is correct, the loop annotation corresponds to a loop pragma, and the scalable ramp/vector node to an IR extension. Like I wrote before, I think there's place for both of these concepts.
Can you please advise how to proceed? I guess we need to document this in the RFC. But do we want to prototype this too? Or do we just list it as an alternative, and discuss here what we would like to do first?
I have not yet commented on your remark about the ABI convention, as I am not familiar enough with TVM to comment about this. It looks though that is a solvable problem.

The ABI issue is important since it affects the DLPack, so I would suggest we agree on the encoding convention before we proceed. I am fine with having a scalable vector repr if we can take the right encoding.

DataType and DLDataType are part of DLPack standard,

The DataType type is defined in TVM, it's not present in DLPack.  Or am I missing something?

Hi @tqchen @kparzysz-quic @kparzysz-quic @masahi @tkonolige @smeijer1234 ,
We are looking to revive this work. I have gone through the thread.
Summary so far is as follows :

We want to introduce/enhance a scheduling vectorization primitive that could be controlled by user/auto-tuner/auto-scheduler either to use scalable vectors in the backend codegen.

The conversation has resolved to be extending the existing vectorize scheduling primitive i.e. : s[C].vectorize(..., scalable=True)

Usage of this scheduling primitive should result in creating a for loop with a Ramp nodes with either an additional argument "is_scalable" or special number for lanes.

I think @tqchen was suggesting to use the special lane number (-1) as opposed to introducing an additional argument to all TIR nodes such as Ramp and Broadcast as well as DataType (and to DLDataType) to avoid ABI breakages.
Moreover, VectorizeLoopScalable will be modified to create a While node.

The name of the RFC is confusing ? @kparzysz-quic . I suppose for TIR, what we are adding is vector-length agnostic vectorization support for TIR, while demonstrating the codegen of VLA vectorized TIR using Arm(R) SVE codegen.

Please confirm whether this is a right summary of the current state.
As for next steps, I would like to propose/resolve each of the outstanding discussion points and update the RFC.

Thanks for bringing up. this is also very useful on RISC-V. we look forward to progress in this regard.

To reiterate---my original concern was that the first RFC was proposing changes to target-independent part of TVM to add support for a very target-specific feature.  However, I do think that we can move this forward in way that would be overall useful.
Here is the outline of my thoughts on this.  Let me know what you think.
First, a couple of observations:

Architectures that support vectors can be assumed to also support vector predication.  I'm talking specifically about masked operations, and in particular about predicated loads and stores.
For ARM/AArch64, it may be beneficial to distinguish vectorization via fixed-length vectors from one via scalable vectors.  If this choice is to be made by auto-scheduling, it should be expressible in TIR.

What this RFC proposes is very close to allowing vectorization of countable loops with variable iteration count, and I insist that we keep this in mind as a goal.
The way that vectorization works right now is that a loop like
for (i : [0, 130)) {
  C[i] = A[i] + B[i]
  D[i] = A[i] * B[i]
}

will be replaced with statements
C[Ramp(0, 1, 130)] = A[Ramp(0, 1, 130)] + B[Ramp(0, 1, 130)]
D[Ramp(0, 1, 130)] = A[Ramp(0, 1, 130)] * B[Ramp(0, 1, 130)]

The expressions within these statement are all PrimExpr, whose type must be expressible by DataType.  All parameters in DataType are compile-time integers, which means that a single statement can only represent vectors with a known number of lanes.  In other words, neither VIC nor VLA can be implemented without some changes.  These changes may be in how types are represented in DataType, or in how vectorization is done (or a combination of these two).
We are already considering a special value for DataType::lanes that would represent the yet-unknown vector length (VL).  Following Halide's approach to vectorization, I propose that we change vectorization to take an explicit vector length as a parameter.  As a special case for SVE, the scalable VL could be represented by the same constant we chose for DataType::lanes.  For compatibility with existing code, stage.vectorize() would be equivalent to stage.vectorize(vector_length=iter_count), since currently only loops with known iteration count can be vectorized.  The argument value vector_length=VL would indicate using SVE.  With vectorize(vector_length=32), the loop above would be turned into
for (i = [0, (130+31)/32) {
  // i-th vector is [32*i..32*(i+1))
  C[Ramp(32*i, 1, 32), pred=(Ramp(32*i, 1, 32) < Broadcast(130, 32))] = A[Ramp..., pred=...] + ...
  ...
}

If the loop iteration count changed from a known integer 130 to some expression N, the generated code would remain mostly the same: the structure does not depend on the fact that 130 is a compile-time constant.  Similarly the 32 indicating vector length could be replaced with the predefined value for "scalable vector length", with the only issue potentially with calculating the iteration count of the for loop above.  If we were to allow an explicit "stride" to For, the issue would go away (the RFC proposes something like that).
To summarize:

Introduce kScalableVectorLaneMark (as suggested by @tqchen).
Make vector length a parameter to stage.vectorize.
Introduce "predicate" to BufferLoad and BufferStore.
Allow non-unit strides in For loops (as per the RFC).

Hi~ here are my two questions :)
cc @kparzysz-quic

2. Make vector length a parameter to stage.vectorize.

What is the different between

sch[C].vectorize(v, vector_length=32) and
vo, vi = sch[C].split(v, 32) then sch[C].vectorize(vi)

It seems that we could also choose to properly lower the split's predicate to reach the same goal as proposed below. For example,  weapons introduced in RFC https://github.com/apache/tvm-rfcs/pull/77 may help?

3. Introduce "predicate" to BufferLoad and BufferStore.

Our team also get confused on how to represent predicated ld/st, when several months ago the upstream upgrade T.load/T.store (who have 1D predicate field) to BufferLoad/BufferStore. Now since BufferLoad/BufferStore are multi-dimensional, the predicate seems to also be multi-dimensional predicate?
Another concern is whether embedding predicate into BufferLoad/BufferStore increase the complexity (or break) buffer region related analysis in existing implementations. Could we leverage T.select(pred, A[...], undef) to represent A[..., pred], or just match the predicated memory access pattern like if (pred) C[...] = ...?

Thanks!

Hi~ here are my two questions :) cc @kparzysz-quic
What is the different between
  * `sch[C].vectorize(v, vector_length=32)` and
  * `vo, vi = sch[C].split(v, 32)` then `sch[C].vectorize(vi)`

  It seems that we could also choose to properly lower the split's predicate to reach the same goal as proposed below. For example,  weapons introduced in RFC [[RFC] Buffer Layout Padding #77](https://github.com/apache/tvm-rfcs/pull/77) may help?

Yes.  The padding proposed in the padding RFC could be utilized for this.  What I wrote didn't take that into account.

  Our team also get confused on how to represent predicated ld/st, when several months ago the upstream upgrade `T.load`/`T.store` (who have 1D predicate field) to `BufferLoad`/`BufferStore`. Now since `BufferLoad`/`BufferStore` are multi-dimensional, the predicate seems to also be multi-dimensional field?

I suppose it could be multi-dimensional, but effectively it would be the conjunction of all the per-dimension predicates.

  Another concern is whether embedding predicate into `BufferLoad`/`BufferStore` increase the complexity (or break) buffer region related analysis in existing implementations. Could we leverage `T.select(pred, A[...], undef)` to represent `A[..., pred]`, or just match the predicated memory access pattern like `if (pred) C[...] = ...`?

The region analysis could simply ignore the predicate (assume that it's true for all lanes).  This is presumably what would happen with the select.  For stores, we'd have to have an if statement that accepts a vector of booleans to properly emulate the store predicate.  Adding a predicate to BufferLoadNode and BufferStoreNode is a feature that could have some value on its own, so perhaps it deserves its own RFC...

@tqchen just to clarify, do we then add this to the DLPack repo or do we consider this a specialized use of lane internal to TVM?

Closing as superseded by: https://github.com/apache/tvm-rfcs/pull/104


merge_deploy_mode = true don't update the current branch deployed

Details
Hi!
Every time a Pull Requests is merged, I'm deploying the stable branch to my development environment.
To do that, I'm using the property merge_deploy_mode = true and it works pretty well. However, it doesn't update the current branch deployed.
If I have an opened PR, it still shows as "deployed" even when it's not:

Is there any way to fix that?

Ok, I forgot to pass environment to the job. Using something like this worked:
jobs:
  deploy:
    name: 'Deploy'
    environment: development
    runs-on: ubuntu-latest
    steps:
      - name: Branch Deploy
        id: branch-deploy
        uses<EMAIL_ADDRESS>        with:
          merge_deploy_mode: "true"
          environment: development
# ...

Glad that you were able to get it all sorted! It should be noted that there are some known bugs with the "Merge Deploy Mode" and I'm still trying to figure out how to resolve the issues.
https://github.com/github/branch-deploy/issues/184
Looks like you got your specific issue resolved but just wanted you to be aware since I can see you are trying it out. Thanks!


problem with VTK on Windows

Hello, I have Anaconda on Windows 10 and I have problems installing PVGeo.
Anaconda is up-to-date: Anaconda 3 2019.10 with Python 3.7.4.
I have installed VTK with: conda install -c conda-forge vtk
All other requirements seem OK and I have installed pyvista without any problem (with conda).
When I try to install PVGeo I get an error saying that the VTK package version cannot be resolved, and so PVGeo cannot be installed:
Installing collected packages: vtk, pvgeo
  Found existing installation: vtk -PKG-VERSION
ERROR: Cannot uninstall 'vtk'. It is a distutils installed project and thus we cannot accurately determine which files belong to it which would lead to only a partial uninstall.

Might be time I get PVGeo and omfvista on conda-forge then


Reduce the number of cells to 24 in Ireland

What
As more tenants have migrated over to London we have a surplus of cells in Ireland, this commit removes another 3. Further reductions will take place over successive PRs.
How to review
Code review alongside User impact dashboard
Who can review
anyone

Closed - not necessary right now - could be reviewed in a week or so


Describe API endpoints for settings

/settings
/settings/webhooks
/settings/email
/settings/email/templates
/settings/email/templates/order_confirmation


plugins/lastplace: add plugin + test

Add plugin nvim-lastplace.

Fulfills https://github.com/pta2002/nixvim/issues/320


fix: various fixes to ensure that the v2 writer is used by the ray sink if requested

Codecov Report
Attention: Patch coverage is 42.85714% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 81.03%. Comparing base (99008c6) to head (2de16d1).
Report is 1 commits behind head on main.

Files
Patch %
Lines

.../lance-encoding/src/encodings/logical/primitive.rs
50.00%
2 Missing :warning:

rust/lance-file/src/v2/writer.rs
0.00%
2 Missing :warning:

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2248      +/-   ##
==========================================
- Coverage   81.06%   81.03%   -0.03%
==========================================
  Files         186      186
  Lines       53745    53753       +8
  Branches    53745    53753       +8
==========================================
- Hits        43566    43560       -6
- Misses       7704     7715      +11
- Partials     2475     2478       +3

Flag
Coverage Δ

unittests
81.03% <42.85%> (-0.03%)
:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


endless reconciles of namespaceconfig CR

We use this https://github.com/redhat-cop/namespace-configuration-operator with this CR:
apiVersion: redhatcop.redhat.io/v1alpha1
kind: NamespaceConfig
metadata:
  name: resource-quota-small
spec:
  labelSelector:
    matchLabels:
      fss.tietoevry.com/resource-quota: small
  templates:
  - objectTemplate: |
      apiVersion: v1
      kind: ResourceQuota
      metadata:
        name: compute-resources
        namespace: {{ .Name }}
        labels:
          app.kubernetes.io/managed-by: namespace-configuration-operator
      spec:
        hard:
          requests.cpu: "2"
          requests.memory: 4Gi
          limits.cpu: "6"
          limits.memory: 4Gi

but the kustomize controller seems to apply it on each run, causing notifications every 10mins.
Running flux v0.19.0

I noticed a diff between the before and after snapshots you posted:
  templates:
  - excludedPaths:
    - .spec.replicas
    - .metadata
    - .status

Arrays can be merged sometimes with strategic merge patches, but when they are not a well-known type like the PodTemplate in Deployment resources, with a well-known key field (name is the key field in the case of the containers array), the strategic merge may not be possible.
Thanks for following this up with us on the Slack channel, (I'm just re-posting here from the progress we made toward understanding the issue here out of that conversation thread.) 👍
Resolving that diff in git by hand may ensure the resource is not changed again and again when Flux reconciles it every spec.interval.

Indeed adding these fields made KC happy and there is no more reconciliation spam.
Also FYI @raffaelespazzoli

I guess we just close this since there isn't much for KC to offer in this case.


Feat/resize pattern

clean up the preview pane ui and make resize elements visible on hover.
let me know what you think @tilmx

I really like the styling and the UX itself. The only thing I am a bit unsure is about if it is so good to have those spaces not clickable for the preview content itself. So e.g. if on the website there is something to be clicked on the left or right side, then you can’t click that any more because of that handles. But let’s talk later about that @markusoelhafen!


Unexpected behaviour arises when hip or chest rotated too much up

Euler angles are a pain.
Took code from BetterDirections to fix it with head locomotion, but it doesn't work perfectly with hip and chest locomotion.
Forward will still be forward and vice-versa, but left and right will get mixed up.

solved by putting the transform.right and transform.forward's y value to 0 instead of using Vector3.ProjectOnPlane


fixes typo in scpch check in ingress

Codecov Report
Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (15179c5) 95.27% compared to head (ea01004) 95.27%.
Report is 2 commits behind head on main.

Files
Patch %
Lines

lac_validator/ingress.py
0.00%
1 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #756   +/-   ##
=======================================
  Coverage   95.27%   95.27%
=======================================
  Files         313      313
  Lines        7717     7717
=======================================
  Hits         7352     7352
  Misses        365      365

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


Remove the deps.rs badge

Closes #179
Unfortunately, deps.rs is no longer maintained and unable to report the status for this crate. See https://github.com/srijs/deps.rs/issues/41 for details.

Codecov Report

Merging #187 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #187   +/-   ##
=======================================
  Coverage   83.00%   83.00%
=======================================
  Files          12       12
  Lines         453      453
=======================================
  Hits          376      376
  Misses         77       77

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c88eb48...abd19c9. Read the comment docs.


Question: Merge Ditflo with Ditto?

Given the small size of Ditflo and the potential confusion people not knowing what it is compared to Ditto, I wonder if we should just merge them together and it's all just Ditto? You can of course still use ditto without using the Ditflo stuff (obviously this would be renamed to just be Ditto on merge), it just puts it all in one library so there is no need to explain the difference.

:100: from me.
I see the DitFloView directive as complimentary to the .As<T> extension method. It is another entry point for Ditto.

Closing, see PR #164 instead


Tables not being created as Temporal

Our project was working with all new tables created Temporal.
We have  modelBuilder.UseTemporalTables(); in our OnModelCreating.
But now for some reason tables created with  migrationBuilder.CreateTable() are no longer created as Temporal.  How can we debug this?

we were missing options.UseInternalServiceProvider(provider);

this is happening again.  we do have options.UseInternalServiceProvider(provider); this time.


ImportError: librocblas.so.0: cannot open shared object file: No such file or directory

Hello, I get this error when I want to use tenserflow rocm
   from tensorflow.python._pywrap_tensorflow_internal import * ImportError: librccl.so.1: cannot open shared object file: No such file or directory
but I don't have this file in opt/rocm/lib
cmake                          libhipblas.so           libhip_hcc.so        libhiprtc.so        libmcwamp_hsa.so  librocblas.so.0         librocfft-device.so.0        librocfft.so.0        librocr_debug_agent64.so
libclang_rt.builtins-x86_64.a  libhipblas.so.0         libhip_hcc_static.a  libmcwamp_atomic.a  libmcwamp.so      librocblas.so.<IP_ADDRESS>  librocfft-device.so.<IP_ADDRESS>  librocfft.so.<IP_ADDRESS>
libhc_am.so                    libhipblas.so.<IP_ADDRESS>  libhiprand.so        libmcwamp_cpu.so    librocblas.so     librocfft-device.so     librocfft.so                 librocrand.so

And when I try to install rocm-libs I get this error
sudo apt install rocm-libs
[sudo] password for server:
Sorry, try again.
[sudo] password for server:
Reading package lists... Done
Building dependency tree
Reading state information... Done
rocm-libs is already the newest version (2.6.22).
0 upgraded, 0 newly installed, 0 to remove and 3 not upgraded.
2 not fully installed or removed.
After this operation, 0 B of additional disk space will be used.
Do you want to continue? [Y/n] Y
Setting up rock-dkms (2.6-22) ...
Removing old amdgpu-2.6-22 DKMS files...

------------------------------
Deleting module version: 2.6-22
completely from the DKMS tree.
------------------------------
Done.
Loading new amdgpu-2.6-22 DKMS files...
Building for 5.4.0-91-generic
Building for architecture x86_64
Building initial module for 5.4.0-91-generic
autoreconf: Entering directory `.'
autoreconf: configure.ac: not using Gettext
autoreconf: running: aclocal --force
autoreconf: configure.ac: tracing
autoreconf: configure.ac: not using Libtool
autoreconf: running: /usr/bin/autoconf --force
autoreconf: running: /usr/bin/autoheader --force
autoreconf: configure.ac: not using Automake
autoreconf: Leaving directory `.'
ERROR: Cannot create report: [Errno 17] File exists: '/var/crash/rock-dkms.0.crash'
Error! Bad return status for module build on kernel: 5.4.0-91-generic (x86_64)
Consult /var/lib/dkms/amdgpu/2.6-22/build/make.log for more information.
dpkg: error processing package rock-dkms (--configure):
 installed rock-dkms package post-installation script subprocess returned error exit status 10
No apport report written because the error message indicates its a followup error from a previous failure.
                                                                                dpkg: dependency problems prevent configuration of rocm-dkms:
 rocm-dkms depends on rock-dkms; however:
  Package rock-dkms is not configured yet.

dpkg: error processing package rocm-dkms (--configure):
 dependency problems - leaving unconfigured
Errors were encountered while processing:
 rock-dkms
 rocm-dkms
E: Sub-process /usr/bin/dpkg returned an error code (1)

here are the contents of /var/crash/rock-dkms.0.crash
ProblemType: Package
DKMSBuildLog:
 DKMS make.log for amdgpu-2.6-22 for kernel 5.4.0-91-generic (x86_64)
 Sun 12 Dec 2021 07:45:54 PM UTC
 make: Entering directory '/usr/src/linux-headers-5.4.0-91-generic'
   CC [M]  /var/lib/dkms/amdgpu/2.6-22/build/amd/amdkcl/kcl_drm.o
   CC [M]  /var/lib/dkms/amdgpu/2.6-22/build/amd/amdkcl/main.o
   CC [M]  /var/lib/dkms/amdgpu/2.6-22/build/amd/amdkcl/symbols.o
   CC [M]  /var/lib/dkms/amdgpu/2.6-22/build/scheduler/sched_main.o
   CC [M]  /var/lib/dkms/amdgpu/2.6-22/build/ttm/ttm_memory.o
   CC [M]  /var/lib/dkms/amdgpu/2.6-22/build/amd/amdgpu/amdgpu_drv.o
   CC [M]  /var/lib/dkms/amdgpu/2.6-22/build/amd/amdgpu/amdgpu_device.o
   CC [M]  /var/lib/dkms/amdgpu/2.6-22/build/ttm/ttm_tt.o
 In file included from /var/lib/dkms/amdgpu/2.6-22/build/ttm/backport/backport.h:7,
                  from <command-line>:
 /var/lib/dkms/amdgpu/2.6-22/build/include/kcl/kcl_reservation.h:4:10: fatal error: linux/reservation.h: No such file or directory
     4 | #include <linux/reservation.h>
       |          ^~~~~~~~~~~~~~~~~~~~~
 compilation terminated.
 make[2]: *** [scripts/Makefile.build:270: /var/lib/dkms/amdgpu/2.6-22/build/ttm/ttm_memory.o] Error 1
 make[2]: *** Waiting for unfinished jobs....
   CC [M]  /var/lib/dkms/amdgpu/2.6-22/build/scheduler/sched_fence.o
 In file included from /var/lib/dkms/amdgpu/2.6-22/build/amd/amdgpu/../include/../backport/backport.h:11,
                  from <command-line>:
 /var/lib/dkms/amdgpu/2.6-22/build/include/kcl/kcl_reservation.h:4:10: fatal error: linux/reservation.h: No such file or directory
     4 | #include <linux/reservation.h>
       |          ^~~~~~~~~~~~~~~~~~~~~
 compilation terminated.
 make[2]: *** [scripts/Makefile.build:270: /var/lib/dkms/amdgpu/2.6-22/build/amd/amdgpu/amdgpu_drv.o] Error 1
 make[2]: *** Waiting for unfinished jobs....
   CC [M]  /var/lib/dkms/amdgpu/2.6-22/build/scheduler/sched_entity.o
 In file included from /var/lib/dkms/amdgpu/2.6-22/build/amd/amdgpu/../include/../backport/backport.h:11,
                  from <command-line>:
 /var/lib/dkms/amdgpu/2.6-22/build/include/kcl/kcl_reservation.h:4:10: fatal error: linux/reservation.h: No such file or directory
     4 | #include <linux/reservation.h>
       |          ^~~~~~~~~~~~~~~~~~~~~
 compilation terminated.
 make[2]: *** [scripts/Makefile.build:270: /var/lib/dkms/amdgpu/2.6-22/build/amd/amdgpu/amdgpu_device.o] Error 1
 make[1]: *** [scripts/Makefile.build:519: /var/lib/dkms/amdgpu/2.6-22/build/amd/amdgpu] Error 2
 make[1]: *** Waiting for unfinished jobs....
   CC [M]  /var/lib/dkms/amdgpu/2.6-22/build/amd/amdkcl/kcl_fence.o
 In file included from /var/lib/dkms/amdgpu/2.6-22/build/ttm/backport/backport.h:7,
                  from <command-line>:
 /var/lib/dkms/amdgpu/2.6-22/build/include/kcl/kcl_reservation.h:4:10: fatal error: linux/reservation.h: No such file or directory
     4 | #include <linux/reservation.h>
       |          ^~~~~~~~~~~~~~~~~~~~~
 compilation terminated.
 make[2]: *** [scripts/Makefile.build:270: /var/lib/dkms/amdgpu/2.6-22/build/ttm/ttm_tt.o] Error 1
 make[1]: *** [scripts/Makefile.build:519: /var/lib/dkms/amdgpu/2.6-22/build/ttm] Error 2
   CC [M]  /var/lib/dkms/amdgpu/2.6-22/build/amd/amdkcl/kcl_fence_array.o
   CC [M]  /var/lib/dkms/amdgpu/2.6-22/build/amd/amdkcl/kcl_kthread.o
   CC [M]  /var/lib/dkms/amdgpu/2.6-22/build/amd/amdkcl/kcl_io.o
   CC [M]  /var/lib/dkms/amdgpu/2.6-22/build/amd/amdkcl/kcl_reservation.o
   CC [M]  /var/lib/dkms/amdgpu/2.6-22/build/amd/amdkcl/kcl_pci.o
 In file included from /var/lib/dkms/amdgpu/2.6-22/build/amd/amdkcl/kcl_reservation.c:33:
 /var/lib/dkms/amdgpu/2.6-22/build/include/kcl/kcl_reservation.h:4:10: fatal error: linux/reservation.h: No such file or directory
     4 | #include <linux/reservation.h>
       |          ^~~~~~~~~~~~~~~~~~~~~
 compilation terminated.
 make[2]: *** [scripts/Makefile.build:270: /var/lib/dkms/amdgpu/2.6-22/build/amd/amdkcl/kcl_reservation.o] Error 1
 make[2]: *** Waiting for unfinished jobs....
   LD [M]  /var/lib/dkms/amdgpu/2.6-22/build/scheduler/amd-sched.o
 /var/lib/dkms/amdgpu/2.6-22/build/amd/amdkcl/kcl_pci.c: In function ‘amdkcl_pci_init’:
 /var/lib/dkms/amdgpu/2.6-22/build/amd/amdkcl/kcl_pci.c:199:83: warning: passing argument 2 of ‘amdkcl_fp_setup’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
   199 |  _kcl_pcie_link_speed = (const unsigned char *) amdkcl_fp_setup("pcie_link_speed",_kcl_pcie_link_speed_stub);
       |                                                                                ^~~~~~~~~~~~~~~~~~~~~~~~~
 In file included from /var/lib/dkms/amdgpu/2.6-22/build/amd/amdkcl/kcl_pci.c:3:
 /var/lib/dkms/amdgpu/2.6-22/build/amd/amdkcl/kcl_common.h:22:63: note: expected ‘void *’ but argument is of type ‘const unsigned char *’
    22 | static inline void *amdkcl_fp_setup(const char *symbol, void *fp_stup)
       |                                                         ~~~~~~^~~~~~~
 make[1]: *** [scripts/Makefile.build:519: /var/lib/dkms/amdgpu/2.6-22/build/amd/amdkcl] Error 2
 make: *** [Makefile:1762: /var/lib/dkms/amdgpu/2.6-22/build] Error 2
 make: Leaving directory '/usr/src/linux-headers-5.4.0-91-generic'
DKMSKernelVersion: 5.4.0-91-generic
Date: Sun Dec 12 19:45:57 2021
Package: rock-dkms 2.6-22
PackageVersion: 2.6-22
SourcePackage: rock-dkms
Title: rock-dkms 2.6-22: amdgpu kernel module failed to build

Here is my configuration:

kernel: 5.4.0-91-generic
cpu: Intel(R) Core(TM) i5-9600K CPU @ 3.70GHz
gpu: Ellesmere [Radeon RX 470/480/570/570X/580/580X/590]
os: Ubuntu 20.04.3 LTS

Hey, I have a config very close to yours (we don't have the same cpu), I had that issue before ImportError: librocblas.so.0: cannot open shared object file, I solved it by rolling back to ROCm 4.1.
The latest verstion (4.5) is very buggy in this setup, it think it has to do with the linux kernel. ROCm 4.1 works as expected.

Okay, I'll try this version!

It does not work, the installation is done well but the files /opt/rocm/bin/rocminfo
/opt/rocm/opencl/bin/clinfo does not exist and if I install earlier versions like crom-3, I get this error with clinfo: ERROR: clGetPlatformIDs(-1001)

have you checked yourself that the folder /opt/rocm-4.1.0/bin/ contains clinfo? if so then remove rocm inside /opt/ and ln -s to 4.1.0. BTW I've updated to 4.2.0 now in order to use PyTorch from pip
also, Ach9#2154 I'm based in Paris

I have checked at the maint, otherwise I have done your manipulation and now rocm contains clinfo but not the rest, yet when I install rocm it does not send an error

if I install rocm with amdgpu, it also sends no error:
 sudo amdgpu-install --usecase=rocm
Reading package lists... Done
Building dependency tree
Reading state information... Done
linux-headers-5.4.0-91-generic is already the newest version (5.4.0-91.102).
linux-modules-extra-5.4.0-91-generic is already the newest version (5.4.0-91.102).
amdgpu-dkms is already the newest version (1:<IP_ADDRESS>501-1337797).
rocm-dev is already the newest version (<IP_ADDRESS>500-56).
0 upgraded, 0 newly installed, 0 to remove and 3 not upgraded.
````

I redid an installation with amdgpu and now I have 2 rocm folders, and if I do rocm-smi, it works. I guess I have to do sudo apt install rocm-libs miopen-hip rccl and then I can run tenserflow. However I still don't have these 2 files
/opt/rocm/bin/rocminfo
/opt/rocm/opencl/bin/clinfo

Hi @Jourdelune
Thanks for reaching out.
I certainly understood the problem.
As ROCm does not support gfx8 anymore, we can not help you on this.
Request to try on officially supported cards and reach us for official help, we are here to help you.
Thank you.

Ok, is there any way to use these graphics cards with tenserflow to speed up the process? It's a shame to have 3 graphics cards that are useless...


several feeds report that url does not appear to be a valid rss/atom feed

http://www.monitor.hr/rss/
http://www.phoronix.com/rss.php

same issue here

I have done.
Some of feed engines don't accept custom user-agent.
change user-agent to browser's user-agent(chrome or firefox).


Masking via GPU_image?

In iOS I can set a mask (just an 8bit alpha texture) on a CALayer and then render that layer as a solid rect containing either a texture or colour and only the high-alpha regions of the mask will be rendered by the GPU onto the frame buffer.
From what I understand this work would normally be done by a fragment shader. Is there an easier way in SDL_gpu?

I do my masking with a shader, but an easier way would be nice.
For simple rects, you can use clipping instead.  For binary masking, I would consider adding stencil buffer support in some form or providing a built-in masking shader.  You might be able to achieve a masking effect with a blend mode, but that would probably necessitate an intermediate blit (refer to tests/blend-mode).

Hi, thanks for your reply! I'll look into the options and if I can come up with anything generalisable enough to put into SDL_gpu I'll make a pull request.

The new simple-shader-demo has an example of a kind of masking shader.

I'm thinking a stencil interface would be nice and then it's added functionality rather than just making something specifically easier.  It'd be very useful with shaders, too.  I'll consider this further.

Sounds good!

I just saw the bit about the simple-shader-demo, looks great and it's exactly what I needed to get started. Would be nice to have a kind of stencil interface, but the demo puts me in the right path for now. Not sure how I missed this back in July :) Still working on the project though in any case, thanks again for SDL_gpu

Hi @grimfang4, I finally got to trying out the masking shaders. Forgive me if this is a silly question, but how do I get them working with shape rendering?
My mask is the same size as my intended output, so I've boiled the fragment shader down to just this:
in vec4 inColor;
in vec2 texCoord;
out vec4 fragColor;

uniform sampler2D mask_texture;

void main(void)
{
    vec4 maskColor = texture(mask_texture, texCoord);
    fragColor = vec4(inColor.rgb, inColor.a * maskColor.a);
}

What I'd like to do is something like this:
GPU_ActivateShaderProgram(mask_shader, &mask_block);
GPU_RectangleFilled(target, rect, sdlColor);
GPU_ActivateShaderProgram(0, NULL);

And have only the masked regions show up in the chosen color. The code as it's written doesn't appear to provide any input to the shader though. Do you have an idea how I could do this with SDL_gpu?

Ok, so I figured out a way of doing what I need as follows:
in vec4 originalColour;
in vec2 absolutePixelPos;

out vec4 fragColor;

uniform float maskMinX;
uniform float maskMinY;
uniform float maskWidth;
uniform float maskHeight;
uniform sampler2D maskTexture;

void main(void)
{
    vec2 maskCoordinate = vec2(
        ((absolutePixelPos.x - maskMinX) / maskWidth),
        ((absolutePixelPos.y - maskMinY) / maskHeight)
    );

    vec4 maskColour = texture(maskTexture, maskCoordinate);
    fragColor = vec4(originalColour.rgb, originalColour.a * maskColour.a);
}

... with absolutePixelPos just being the gpu_Vertex passed into the default vertex shader.
This works, but it feels weird to be working with in absolute texel positions (including maskMinY) etc.
I noticed that DoUntexturedFlush doesn't set the texcoord AttribArray like DoPartialFlush does. If it did (and it's quite possible I'm misunderstanding this), I get the impression these absolute pixel positions could go away (including the maskWidth/Height) and I'd be left with the very simple code snippet above.
It seems valuable to allow shaders to work on shapes generally, so would you accept a change along those lines in a pull request?

I am going to try to change the render code to use the matrix stack instead of blitting absolute positions, I think this may fix the issue I'm having. Sorry for spamming, but this may be useful for someone else having the same issues.

On the topic of stencil buffers I've been trying to track down some issues relating to NanoVG and I suspect SDL_gpu lacking a stencil buffer might be the cause. Am I wrong for thinking there is no stencil buffer initialized? Would it be possible to do this without major code changes, unrelated to the GPU_Image struct itself.


Config files - beacon

Hi, just a query.
Is there any reason beacon manager would have issues, it basically just writes to the config files but I can't get breeding edits to work, does the container do anything other than pass the files in the config pass through.
Also how does it build the start command? Can I add "text" and - flags to the extra args section without issue?
Mant thanks

yeah if you dont want to use official server settings make sure to remove -UseDynamicConfig from  - CUSTOM_SERVER_ARGS=-UseDynamicConfig so it should look like this - CUSTOM_SERVER_ARGS= and you can do -edit to remove it from the docker-compose.yaml file

Great thanks, I saw another post on here that helped me solve it. Love the docker container and POK manger very helpful, even doing a - custom Dino wipe so easy.
Thank you


python3Packages.aggdraw: 1.3.16 -> 1.3.18

Description of changes
Fixes Python 3.12 build for #299993, supersedes #298455
The patch I removed was added in https://github.com/NixOS/nixpkgs/commit/3e7992ba67ff4d6424879f2636857866229c952c because there was no upstream release that contained this fix yet. Another issue related to Python 3.12 deprecating some APIs also came up while testing #299993, which is fixed by aggdraw version 1.3.18.
Things done

Built on platform(s)

[x] x86_64-linux
[ ] aarch64-linux
[ ] x86_64-darwin
[ ] aarch64-darwin

For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)

[ ] sandbox = relaxed
[ ] sandbox = true

[ ] Tested, as applicable:

NixOS test(s) (look inside nixos/tests)
and/or package tests
or, for functions and "core" functionality, tests in lib/tests or pkgs/test
made sure NixOS tests are linked to the relevant packages

[ ] Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
[ ] Tested basic functionality of all binary files (usually in ./result/bin/)
24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module

[x] Fits CONTRIBUTING.md.

Add a :+1: reaction to pull requests you find important.

Result of nixpkgs-review run on x86_64-linux 1

6 packages failed to build:

hydrus
hydrus.doc
python311Packages.psd-tools
python311Packages.psd-tools.dist
python312Packages.psd-tools
python312Packages.psd-tools.dist

4 packages built:

python311Packages.aggdraw
python311Packages.aggdraw.dist
python312Packages.aggdraw
python312Packages.aggdraw.dist


Can't get script block to work

I seem to be unable to get unbuffred script blocks to work, this simple test:
-
  console.log('hello world');

h1 Hello World

Results in this error:
	Page Load Error.
	testa
	Error: /usr/share/nodejs/testa.jade:1 > 1
	- 2
	console.log('hello world'); 3
	4
	unexpected text - c

Yet everything works fine if I use single lines of code - What am I doing wrong ??

It's most possible that you are not using the current version of Jade/Pug. Use npm ls jade to check what version you have.

I'll close this until the necessary information is provided. I will then reopen this ticket.

Can confirm this bug. Example: https://github.com/jimmytheneutrino/testjadescriptblock

https://github.com/jimmytheneutrino/testjadescriptblock/commit/197317b27e493d1f3a09933058cd655cc89628f4#diff-4f6ff00ce4f43d88fb822cc1482b80dcR2

Ok, removing the space fixes it. Thanks!
But maybe this behaviour could be documented or
instead be a little bit more forgiving?


what happened?  the problem about react-native init

My enviroment like this
$nvm ls
v6.2.1
->       system
default -> system
node -> stable (-> v6.2.1) (default)
stable -> 6.2 (-> v6.2.1) (default)
iojs -> iojs- (-> system) (default)
when i use react-native init --verbose  to create a project
├──<EMAIL_ADDRESS>├──<EMAIL_ADDRESS>├─┬<EMAIL_ADDRESS>│ └──<EMAIL_ADDRESS>└──<EMAIL_ADDRESS>
npm WARN<EMAIL_ADDRESS>requires a peer of<EMAIL_ADDRESS>but none was installed.
npm verb exit [ 0, true ]
npm info ok
then,it no response!
I tried to install<EMAIL_ADDRESS>and then retry init
the same problem
only has node_modules and package.json in MyProject
what happened

I can't reproduce that issue.  Maybe you need to install it globally?

Has Done!

I uninstalled the nodejs 6.2.2
and reinstall the 4.4.5 version
init success...


bug: Lazy restore update plugin's hash

Did you check docs and existing issues?

[X] I have read all the lazy.nvim docs
[X] I have updated the plugin to the latest version before submitting this issue
[X] I have searched the existing issues of lazy.nvim
[X] I have searched the existing issues of plugins related to this issue

Neovim version (nvim -v)
NVIM v0.10.1
Operating system/version
fedora 40
Describe the bug
after restore plugin, some plugins's hash was updated.
Steps To Reproduce

clone your nvim config
run Lazy restore
lock file changed and some plugins's hash was updated.

Expected Behavior
hash shouldn't update
Repro
No response

It's not restore() updates lockfile. Lockfile is updated by setup(), before we get a chance to call restore().
Here is how to reproduce this:

Have lazy and plugin A installed.
Have lockfile which declares commit hashes for lazy, plugin A and plugin B - and these hashes did not match current HEAD for both lazy and plugin A.
vim.o.loadplugins=true
Call require('lazy').setup(...) with spec= contains plugin A and plugin B.

The result will be:

Plugin B will be correctly installed using commit hash from a lockfile.
Lockfile will be updated with current HEAD hashes for all plugins, which means hashes for lazy and plugin A will change.

This behaviour of setup() breaks use case when we wanna install on CI all plugins using versions from lockfile:

Have a lockfile with commit hashes for all plugins including lazy itself.
Have no plugins installed (and no lazy too).
Bootstrap lazy (it will clone latest main branch).
vim.o.loadplugins=true
require('lazy').setup({spec={all plugins here}}). All these plugins (except the lazy itself) are missing, so setup() will both install them using correct commit hashes from lockfile and update lazy's hash in the lockfile to current HEAD (i.e. latest main).
Then we call restore() which will do nothing while we expect it to restore lazy version to the one from original lockfile (before it was modified by setup().

To fix this issue setup() should be modifed to avoid changing existing hashes in a lockfile.
It still should modify a lockfile to add hashes for new plugins it just installed, of course.

Actually setup() does this because of default option install = { missing = true }. But disabling it does not solve the issue - we anyway need to choose what to call first: restore() (which will restore existing plugins to commit hashes from lockfile but also it'll remove commit hashes for non-existing plugins from lockfile) or install() (which will install missing plugins using commit hashes from lockfile but then update lockfile and set commit hashes for existing plugins to their current HEAD).
Looks like right now we can't get both and apply commit hashes from lockfile to both existing and missing plugins. :disappointed:

bump

Yep. This was happening for a long time. I use same config in two computers and when I restore the new changes in the other, it always show changes in lock file though I'm only restoring

Yep. This was happening for a long time. I use same config in two computers and when I restore the new changes in the other, it always show changes in lock file though I'm only restoring

@s1n7ax i have a similar setup as well. I created a post-merge git hook that just runs nvim --headless "+Lazy! restore" "+Lazy! clean" +qa. This seems to have provided a temporary fix for me.


Provide final content edits

After all else, do a final pass on the site, updating any placeholder content, editing what's there, and giving it a final sign-off.

We're still figuring out what's going on with a few fields, but the descriptions for the schema table are here: https://docs.google.com/spreadsheets/d/1ZiV8-ks4AL2wqeKKkcuoI84GdMRZUE8xSQ87tCh2-8k/edit?usp=sharing

Tiny little edits, but on the Guide to the Data page:
In the MediaReview box: "interested in tagging their articles with ClaimReview [ADD: and MediaReview]"
In the Tips box: Under "A ratings exception": Link to the full MediaReview Ratings on either the phrase "standard ratings" or "Those ratings"
https://www.claimreviewproject.com/mediareview/definitions-and-examples


[Feature Request]: Market Price Prediction for Crops in India

Is there an existing issue for this?

[X] I have searched the existing issues

Feature Description
I would like to propose adding a Market Price Prediction feature to AgroTech.ai. This feature would use historical market trends, weather patterns, and demand-supply data to predict future crop prices, helping farmers make more informed decisions about when to sell their produce. We can integrate publicly available datasets like the APMC Market Price dataset or FAOSTAT data to build this model.
Use Case
This feature could greatly benefit users by providing insights into market fluctuations and maximizing their profit margins.
Benefits
No response
Add ScreenShots
No response
Priority
Medium
Record

[X] I have read the Contributing Guidelines
[X] I'm a GSSOC'24 contributor
[X] I have starred the repository

hi @AYUSHI-SHA  if you can create frontend and api.. i will put labels here..because without froontend there is no use of it..and causing problems in cloning for others..please let me know if you can create frontend part...as of now you are simply getting projects from somewhere in other repos and pasting them here...im sorry for removing labels..if you do fronend and api part i can relabel them ..i think you understand my point...simply having notebook files does nothing..until now..which are in notebooks..all of them have frontend and api..
thank you @AYUSHI-SHA

if you have any queries connect me here: https://www.linkedin.com/in/manikumarreddyu


Suggestion: Move to JS modules for better portability

Using JS modules would allow better portability between other runtimes, like Deno.

as of the latest release, the code is now using esmodules :3


There is not an easy way to tell what filters are applied after they've been removed from the sidebar

Here's an example:

It said there is one filter applied, but there's no way to tell which one it is. You have to add each variable to the sidebar until you find it.
I think a good way to fix this is to force a variable that has an active filter to always remain in the sidebar. If the filter is active, we can replace the "x" icon that would remove it to a "minimize" icon, "-", which would collapse the filter so that it only shows the filter header (name and label) and maybe a short summary of the filter (kind of like the pills that used to be at the bottom of the app). Then the user could expand the variable to see the full filter view. If they clear the filter, then the minimize button would become the "x" remove button.

we decided to do a modal for confirming and whenever a filter is removed it clears it.

And the modal should only appear if there is an active filter on the variable that has been closed. Otherwise, just close.


feat(codecov): add hooks to travis build

Current coverage is 46.66%

Branch #549 has no coverage reports uploaded yet.

No diff could be generated. No reports for master found.

Powered by Codecov. Updated on successful CI builds.


EscapedNotTranslated: use PHPCSUtils GetTokensAsString

The PHPCS native getTokensAsString() method has two drawbacks:

It is fiddly to use as you always need to do some calculation to find the number of tokens you want to method to concatenate.
It will concatenate everything, i.e. including new lines, comments etc.

The getTokensAsString() method is often used to display a "found" string in an error message and the error message would benefit from that "found" string being clean of comments and having whitespace compacted.
In comes PHPCSUtils. PHPCSUtils has dedicated GetTokensAsString class which contains a variety of methods to retrieve a concatenated string of the content of a group of tokens.
So, for the EscapedNotTranslated sniff, replacing the function call to the PHPCS native method with one to one of the PHPCSUtils variations, ensures that the "found" string in the error message is clean and easy to read.
Tested by adjusting one of the existing unit tests.
The current unit test setup does not allow for testing the actual error message, but you can see the difference if you just run the sniff (with and without the change) over the adjusted unit test case file.

The errored build is because of: [Composer\Downloader\TransportException] Peer fingerprint did not match. This can be ignored?

@dingo-d I just retriggered that build. It should pass.


RuntimeError:CUDA out of memory

Can I remove the attention layer for high resolution img?

Yes you can try removing the attention to see if that gets rid of the error. Whats the image size you are working with ?
Adding a bunch of things that will reduce the compute requirement in case you are working with the default config.

Reduce Batch Size(config uses 64 as of now)
Keep attention only in midblock and remove Downblock attention(here) and Upblock attention(here)
By default the downsampling is disabled on last downblock(since mnist images are anyways small), so change this value to be [True, True, True]

Yes you can try removing the attention to see if that gets rid of the error. Whats the image size you are working with ?
Adding few other things that will reduce the compute requirement in case you are working with the default config.
1. Reduce Batch Size(config uses 64 as of now)

2. Keep attention only in midblock and remove Downblock attention([here](https://github.com/explainingai-code/DDPM-Pytorch/blob/main/models/unet_base.py#L98-L105)) and Upblock attention([here](https://github.com/explainingai-code/DDPM-Pytorch/blob/main/models/unet_base.py#L275-L281))

3. By default the downsampling is disabled on last downblock(since mnist images are anyways small), so change [this](https://github.com/explainingai-code/DDPM-Pytorch/blob/main/config/default.yaml#L14) value to be `[True, True, True]`

Thank you for your advice,I use img with size 900*1600. That consume huge memory when encounter attention layer.

Got it. Yeah try with a batch size of 1 first.
If it works then you can train with gradient accumulation.
But if that also fails then you would have to either remove attention layers or train with smaller sized images.

Got it. Yeah try with a batch size of 1 first. If it works then you can train with gradient accumulation. But if that also fails then you would have to either remove attention layers or train with smaller sized images.

Thank you ,I will try as you told.


fix title: 'Request' -> 'Transaction'

Somehow reviewers and I all missed this. The h1-level title in the doc should match the file name. Fixes that.

+1

LGTM.


Can't run on REAL iPhone5

My flutter app can run successfully on iPhone6 of iOS 12, and a simulator with iPhone8 of iOS 13. However, I tried to run it on my iPhone5 with Xcode and got the error msg is:
Non-fat binary /Users/myUserName/Library/Developer/Xcode/DerivedData/Runner-hgzrgeegogvqfyauuvjkxwdvydmk/Build/Products/Debug-iphoneos/Runner.app/Frameworks/audioplayers.framework/audioplayers is not armv7. Running lipo -info: Non-fat file: /Users/myUserName/Library/Developer/Xcode/DerivedData/Runner-hgzrgeegogvqfyauuvjkxwdvydmk/Build/Products/Debug-iphoneos/Runner.app/Frameworks/audioplayers.framework/audioplayers is architecture: arm64
I don't understand that if this error caused by my wrong configuration or audioplayers is not supported on iPhone5?

From a quick research I did, apparently armv7 architecture (which iPhone 5 uses) is dropped on more recently xcode versions, so that is probably why you are seen this error.
I have found this guide that may help you supporting this device version: https://github.com/flutter/flutter/wiki/Supporting-legacy-platforms


Upgraded nuget packages and added CI/CD workflow

Upgraded Z3 package to 4.8.9
Upgraded CommandLineParse package
Upgraded test packages
Added github CI/CD workflow with status badge

@NikolajBjorner you'll need to approve this since I no longer have write access to Z3prover


how to get stderr

Is there any example to get stderr for expect?

Hi @RickSKy
Thank you for the question.
As far as I understand the question you what to read from stderr right?
expect reads from stdout and stderr simultaneously so there's only 1 way I know how to do it.
You need to turn of stdout.
See this example.
use std::process::Stdio;

use expectrl::{Error, Session, WaitStatus};

fn main() -> Result<(), Error> {
    let mut cmd = std::process::Command::new("sh");
    cmd.arg("-c").arg("echo print to stdout && echo print to stderr >&2");

    // turn off stdout
    cmd.stdout(Stdio::null());

    let mut p = Session::spawn(cmd)?;

    let line = p.expect('\n')?;
    println!("{:?}", String::from_utf8_lossy(line.as_bytes()));

    let line = p.expect('\n')?;
    println!("{:?}", String::from_utf8_lossy(line.as_bytes()));

    assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0));

    Ok(())
}


Fix css inclusion in some plugins

That’s why my image was not included. I copied/pasted your code without reading it.

This breaks the ingress-icon plugin build, because the build system interpret the url() while it shouldn't on https//...
So either the build system should be fixed first, or include_string matches the current use case, and there is nothing to fix

is likely to prevail in order to match mixed url usage, with local pictures and full url.


Competition page hero title text color is wrong

See: https://openproblems.bio/neurips_2022/
Title and subtitle should be white, not black

Fixed by 066933b


Dragging status for Tabablz

Is there anything like IsDraggingWindow but for dragging tabs inside the same Tabablz control? I'd like to limit other UI actions while the user is dragging tabs around to avoid adding or removing new tabs mid-drag.
I've taken a look at PositionMonitor but it seems it only warns about ordering change.

Hi,
If you are using a DragablzItem directly, you can access the readonly dep prop IsDragging:
https://github.com/ButchersBoy/Dragablz/blob/master/Dragablz/DragablzItem.cs#L281
However, most real world scenarios wont be (you will be using some kind of source collection).  In which case you can still listen to the RoutedEvent IsDraggingChangedEvent:
https://github.com/ButchersBoy/Dragablz/blob/master/Dragablz/DragablzItem.cs#L294

Worked great, thanks!


[Enhancement] Allow TabItem's text to not always be uppercase on Android

Summary
Currently, TabItem's on Android default to the Material Design template. That template will always use uppercase to follow Android's Material Design. To be flexible, it would be nice for developers to be able to override this and choose how the text would be displayed.
API Changes
To solve this change, a new binding of type 'TextCaseType' would be added to the TabViewItem class. It would default to Upper to keep the existing logic the same, but allow users to override the value to get their desired effect. This binding would then be used within the MaterialTabViewItemTemplate class to set the Text property's case.
This could also be leveraged for the other templates (iOS, UWP) that are already built, but that may be out of scope for this request.
public static readonly BindableProperty TextCaseProperty =
		   BindableProperty.Create(nameof(TextCase), typeof(TextCaseType), typeof(TabViewItem), TextCaseType.Upper);

public TextCaseType TextCase
{
	get => (TextCaseType)GetValue(TextCaseProperty);
	set => SetValue(TextCaseProperty, value);
}

Intended Use Case
TabItem's using the default Android control template (MaterialTabViewItemTemplate)
Who Will Do The Work?

[x] I am willing to take this on myself
[ ] Just putting this out there for someone to pick up

Thank you for your suggestion @TravyDale however, you are already able to do this.
The default template indeed mimics how Android does things, including the uppercasing, but you can override the whole template and implement something custom.
You can see that here for example: https://github.com/xamarin/XamarinCommunityToolkit/blob/main/samples/XCT.Sample/Pages/Views/TabView/CustomTabsPage.xaml#L12-L35
Does that fit your needs?

Yes it does. I was just trying to avoid doing a custom template. This works just fine though. Thanks for the reply!

Just in case it helps somebody, I'd like to give an example about what I have implemented to avoid uppercase on Android :
<ControlTemplate x:Key="ProductDetailsTabItemTemplate">
        <Label
            x:Name="ProductDetailsTabTitle"
            FontFamily="{TemplateBinding CurrentFontFamily}"
            HeightRequest="{OnPlatform Android='36',
                                       iOS='40'}"
            HorizontalTextAlignment="Center"
            Text="{TemplateBinding Text,
                                   Converter={StaticResource StringCaseConverter},
                                   ConverterParameter=TitleCase}"
            TextColor="{TemplateBinding CurrentTextColor}"
            VerticalTextAlignment="Center" />
        </ControlTemplate>

@jfversluis , you may have noticed that I'm not using the XCT TextCaseConverter 😄
I actually wanted to do a PR to add the "TitleCase" TextCaseType enum, but I failed in running the unit tests in VS for Mac 😕

@jfversluis As @TravyDale says, this use case could be implemented by adding a TextCase property to the TabViewItem.
On Android, it can default to TextCaseType.Upper and on the other platforms to TextCaseType.None so the text stays equal as it is currently.
Though it is a pretty edge case I think it could be simple to implement and I'm willing to do it. What do you think?


MRTK shaders are not updated through Mixed Reality Feature Tool

Describe the bug
When updating a project with MRTK v 2.5.x to v 2.6 preview with the Mixed Reality Feature Tool, the shaders in Assets/MRTK/Shaders are not updated, but the custom inspector script is updated and this is causing issues when selecting a Material with the standard shader (and some other problems).
I think this issue is only partly related to this repository but i dont know where the Mixed Reality Feature Tool is maintained.

Thanks @FlorianJa!
The issue is with how MRTK copies in the shader files. It looks for a specific file and copies if not found. There are currently no version checkss

@davidkline-ms Have the shaders been updated for 2.6? If so, this feels like a ship blocker until developers can successfully upgrade MRTK.

Looks like there are three PRs that are new in 2.6: https://github.com/microsoft/MixedRealityToolkit-Unity/pull/8739, https://github.com/microsoft/MixedRealityToolkit-Unity/pull/8842, https://github.com/microsoft/MixedRealityToolkit-Unity/pull/8812

In progress.
The standard assets package will check for a version number in the sentinel file and prompt for customer approval to overwrite.
Also adding an item in MRTK > Utilities to check for shader upgrade.


Reorganize folders and drop xml for now

Using a simple flat folder structure vs the deeply nested structure in autorest.csharp.
Under src we will have

Expressions - Classes that inherit from ValueExpression
OutputTypes - Types constructed from output library like propertydeclaration, modeltypeprovider, etc
PostProcessing - Classes that deal with roslyn post processing of csharp files
Snippets - Helper classes for expressions / statements
SourceInput - Roslyn existing / custom code representation / converter / helper
StartUp - Start up process for loading plugins and initiating the generation
Statements - Classes that inherit from MethodBodyStatement
Utilities - Any extension on system types or general utilities
Writers - Writers to convert output types into csharp files

API change check
APIView has identified API level changes in this PR and created following API reviews.
Microsoft.Generator.CSharp


Lazy load commands

Also enforce double quotes only for strings, Python line length convention (<120 characters), fix typos, remove unused imports, clean up logic, improve path safety, and fix a few bugs.
Additional improvement is possible by not executing the setup code involving torch and transformers unless it's actually needed, but doing that cleanly would be difficult.
I also did some benchmarks (wall times, median of 20 runs) to see how long it takes to start executing a command (i.e. import everything):

Command
trill 1.6.0
This PR

trill
9102ms
2115ms

trill classify
9109ms
6364ms

trill diff_gen
9101ms
2147ms

trill dock
9097ms
3685ms

trill embed
9080ms
3518ms

trill finetune
9125ms
3660ms

trill fold
9071ms
3560ms

trill inv_fold_gen
9109ms
3587ms

trill lang_gen
9088ms
3548ms

trill simulate
9131ms
2280ms

trill utils
2280ms
2728ms

trill visualize
9076ms
5161ms

Wow, you rock! Thanks for adding them to 1.7.0 too, I'm closeish to releasing it


fix jdbc connect parameters can not input '@' and '.' character

This closes #8135
Purpose of the pull request
fix jdbc connect parameters can not input '@' and '.' character
Brief change log

Verify this pull request
This pull request is already covered by existing tests, such as (please describe tests).
can input '@' or '.' character in jdbc connect parameters

Could you add some unit test to it? @zhuxt2015

Codecov Report

Merging #8293 (c955c3d) into dev (84b004d) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##                dev    #8293      +/-   ##
============================================
+ Coverage     45.29%   45.31%   +0.02%
- Complexity     3997     3999       +2
============================================
  Files           683      683
  Lines         26502    26502
  Branches       2845     2845
============================================
+ Hits          12003    12010       +7
+ Misses        13370    13361       -9
- Partials       1129     1131       +2

Impacted Files
Coverage Δ

...ce/api/datasource/AbstractDataSourceProcessor.java
65.21% <100.00%> (ø)

...lphinscheduler/rpc/common/AbstractRpcCallBack.java
0.00% <0.00%> (-100.00%)
:arrow_down:

...inscheduler/common/task/sqoop/SqoopParameters.java
74.00% <0.00%> (ø)

...he/dolphinscheduler/common/enums/SqoopJobType.java
88.88% <0.00%> (+88.88%)
:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 84b004d...c955c3d. Read the comment docs.

Could you add some unit test to it? @zhuxt2015

yes, I'll add a unit test

Error:  SonarCloud server [https://sonarcloud.io/] can not be reached   @zhongjiajie   I don't know why appear  this error,  can you help me resolve

I restart the failing CI just now

Thanks @caishunfeng for reviewing


Romtable fix & Chip ID

Do you still want to merge this?
I think we found out that unlocking the ROM tables was not what was missing.

This still seems like useful functionality (not that I've actually read the specs...)

Hmm well, we could just add the register defs in another branch. But the unlocking is really not required.


Ender Dragons spawned via Soul shards spawner dont work

Issue Description:
ender dragon will not take damage when spawned form Soul shards spawner (note: I am using a mod called ender totem which prevents any and all ender dragons near by to be drawn in and be unable to move [requires a dragon egg to be right clicked upon the ender totem to function])
What happens:
Ender dragons spawn in,  but cant take any damage
What you expected to happen:
Ender dragons spawns in, and can take damage/be hit
Steps to reproduce:

obtain tier 5 ender dragon soul shard from creative (adjust relevant config option to true)
Place soul shard in soul shard spawner via right click
activate via redstone signal and wait for ender dragons to spawn
atempt to hit ender dragons
...

Affected Versions (Do not use "latest"):

Soul Shards: 1.12-2.7.5-55.
Minecraft: 1.12.2
Forge: 1.12.2-<IP_ADDRESS>18

There isn't really anything I can do about this. Dragons are extremely special cased in Vanilla. Boss mobs are disabled by default because I don't support them.


[NetAppFiles] bug fix allowedClients fix

Related command
az netappfiles volume create
Description
Fix a bug where parameter --allowed-clients was not passed along in the request.
Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change
[Component Name 2] az command b: Add some customer-facing feature

This checklist is used to make sure that common guidelines for a pull request are followed.

[ ] The PR title and description has followed the guideline in Submitting Pull Requests.

[ ] I adhere to the Command Guidelines.

[ ] I adhere to the Error Handling Guidelines.

NetAppFiles

@audunn I've changed the PR header so that it can follow the pattern in azure-cli release history. Please follow this doc for PR title next time. Thanks.


Delete view does not work

No reaction on selecting this menu option.
It would also be nice to have the view menue items available here (after pressing edit on the left pane). That would avoid the need to run the view and produce data befire one can delete or rename the view.

@walter-weinmann Maybe something for you?

I'll try my best.
On 8 March 2016 at 10:27, Stefan Ochsenbein<EMAIL_ADDRESS>wrote:

[image: 2016-03-08 10_19_13-zhhapmop-sbsm01 it bwns ch - remote desktop
connection - __remote]
https://cloud.githubusercontent.com/assets/1941407/13597192/7567c07e-e517-11e5-978a-bb771bd15c23.png
No reaction on selecting this menu option.
It would also be nice to have the view menue items available here (after
pressing edit on the left pane). That would avoid the need to run the view
and produce data befire one can delete or rename the view.
[image: 2016-03-08 10_23_01-zhhapmop-sbsm01 it bwns ch - remote desktop
connection - __remote]
https://cloud.githubusercontent.com/assets/1941407/13597247/c96537ce-e517-11e5-966b-108ff7961cbc.png
@walter-weinmann https://github.com/walter-weinmann Maybe something for
you?
—
Reply to this email directly or view it on GitHub
https://github.com/K2InformaticsGmbH/dderl/issues/175.

--

Walter Weinmann

Obertorplatz 4
CH-4130 Rheinfelden
Tel +41 (0)61 841 06 10

Schulstrasse 1
CH-6037 Root
Tel +41 (0)41 530 39 70

Fixed on PR https://github.com/K2InformaticsGmbH/dderl/pull/182 from @walter-weinmann


Turn based play

It's turn-based play y'all
It works.
Future work:

clear both gameboards when 'Play Again' is clicked
display whose turn it is
disable clicks on board after a winner is found
start text/game data channel even without video call
create button somewhere to restart whole game

Confirmed working of turn based gameplay


Ju 499 fix email validation

JU-499 fix email validation.
Update all javax import to jakarta.
https://companieshouse.atlassian.net/browse/JU-499

CI: No security warnings found


Fix: Polling Interval not being updated to "off" when selecting a queue.

Scenario: From the dashboard, select the polling interval as 3 sec, then select a read-only queue, then you can see that the polling will continue in 3 sec interval, although in the settings modal the polling intervals are hidden and set to off.
Fix: Added a use effect, which will trigger when queue is set to read-only and update the polling interval to off, instead of only hiding the options

Ham... this is weird, since "off" in polling interval means that the board won't fetch updates at all...
This is not the intented behaviour... readOnly mode is ment for cases that the board is given to none thechnical users, which the board should prevent them from making actions, but, the board should update it's data.

I'm closing this PR since setting the poll interval to -1 will disable the update.
More over, I'll revert the previous change, since it will break the default behaviour.


use fnv hasher

In my benchmarks this is twice as fast as the Sip hasher fore only 5 extra lines.

Sweeet, thanks!
If you happen to benchmark some of the parsec stuff, please share the results.


fix: major code restructuring

:tada: This PR is included in version 1.0.0 :tada:
The release is available on:

npm package (@devel dist-tag)
GitHub release

Your semantic-release bot :package::rocket:


Conditional statements referencing values within container don't work

I have a ConditionalContainer to show and hide 'advanced options' for voucher codes and when I edit the Nova resource I want to test the contents of that container and show the container if it has some values set within it.
e.g. I want the following container to show if the min_basket field has a value.
Text::make( __('Code'),  'code')
            ->rules('required')
            ->sortable()
        ,
        ConditionalContainer::make([
            Text::make( __('Min Basket Value'),  'min_basket')
                ->hideFromIndex()
                ->sortable()
        ])->if('min_basket truthy true')

but it doesn't work.
However, if I use the 'code' field in the if statement e.g.
Text::make( __('Code'),  'code')
            ->rules('required')
            ->sortable()
        ,
        ConditionalContainer::make([
            Text::make( __('Min Basket Value'),  'min_basket')
                ->hideFromIndex()
                ->sortable()
        ])->if('code truthy true')

then it does show and hide depending on the content of the field 'code'
Is this not possible to show/hide depending on fields within the container?

It doesn't make sense depend on a field that is within the container itself, in your example min_basket would be hidden at the very first render, how would ever be able to change the value of it?
But even if you had a default value set and somehow it is visible on the form, this package don't check for internal fields.
So it's not an issue, this decision was made by design, I'm not creating nor setting listener for fields that doesn't exist outside the container, so in your case min_basket should have never been visible on the page

Ok, no probs.
Just to let you know the reason for this request - the use case is as follows:
When creating the voucher resource, there are some options that are 'advanced options' which don't need to be visible on creation, unless the user chooses to 'see them'.  There is a checkbox to trigger the show and hide of the container ->if ('advanced_checkbox = 1').
When editing the voucher resource, I want to check if any of the advanced fields have any data in them and if they do - then default the container to be opened. e.g. ->if('advanced_field1 truthy true') ->if('advanced_field2 truthy true') so that the fields can be seen.
So effectively, have a checkbox to show and hide AND show based upon the content within the container.

@milewski I'd like to give this a bump - one of the great potentials for ConditionalContainer is to be able to tidy up large forms into distinct sections that are shown/hidden, so I think you're missing the point of @PaulLawton227's ticket.

Im still not sure exactly sure of what would be the benefits of it.. from my point of view I see something like this happening:
 ConditionalContainer::make([
            Text::make('Test')->withMeta([ 'value' => 'hello world' ])
        ])
        ->if('test === "hello world"')

So taking this example:
1 - The Test field starts with the value hello world so the conditional is met, so it`s visible on the page
2 - if user type anything on the field it will suddenly hide itself as the condition will evaluate to false
3 - There will be no way to bring the field back unless you refresh the page and start the form over again
So I think this is a bad UX
On @PaulLawton227 on your use case, I think you could just use multiple containers? it`s possible to nest containers within itself as long as they depend on outside fields for example:
Checkbox::make('checkbox'),

ConditionalContainer::make([
     Text::make('Demo'),

     ConditionalContainer::make([
         Text::make('Test')
     ])->if('demo contains 123"')

 ])->if('checkbox truthy true"'),


Allowing !eval in help channels

This PR implements the policy change we've agreed upon during the staff meeting: Allowing the !eval command to be used by regular members in help channels. However, as our help channels are dynamically generated up to a certain number of channels, using a simple whitelist based on channel IDs won't work. That's why this PR generalizes the in_channel decorator to be able to whitelist more aspects of the command's context, including the channel category. This means that we can whitelist channels, including help channels, by the categories that they are in.
This PR closes #893
Generalizing the in_channel decorator
The previous decorator, in_channel, allowed us to whitelist commands based on the channel context they were issued in. However, unlike the name suggested, it could also be used to whitelist commands based on another aspect of the command context: The roles the author of the message containing the commands had. In fact, despite the name in_channel, the decorator could in principle be used to whitelist commands solely based on the author's roles using the bypass_roles kwarg.
This means that instead of having an in_channel decorator, we basically had a decorator that whitelisted commands based on two aspects of the command invocation context (i.e. the channel and the author's roles). To highlight this fact, I've generalized the decorator to a general "on_whitelisted_context` decorator that also allows for a third kind of context whitelist: the category containing the channel.
To force readability, I've made it so all the arguments should given as keyword arguments. This means that it's directly clear on which aspects of the context the decorator acts:
@in_whitelisted_context(
    whitelisted_channels=EVAL_CHANNELS,
    whitelisted_categories=EVAL_CATEGORIES,
    whitelisted_roles=EVAL_ROLES,
    redirect_channel=Channels.bot_commands,
)

As it's now no longer obvious if and which redirect channel needs to be used to send a member to in case the check fails, the decorator now takes an optional redirect_channel kwarg. If such a channel is not given, the error message will not mention a "go here instead" channel; if it is given, we'll kindly suggest to the member that they should use the command there. (My guess is that this channel will usually be #bot-commands.)
If we ever want to add more options for context whitelisting, adding another kwarg to the decorator should be trivial.

I dislike splitting up decorator calls into multiple lines and the fact that it's practically impossible to use it as one line now, even for the simplest of calls, rubs me the wrong way.
1. Remove the whitelisted_ prefix from kwarg names. I think redundant given the decorator name.

I don't mind that a decorator spans over multiple lines, as the kwarg names make explicit what each individual kwarg does and it's highly readable. From that perspective, I don't think they're necessarily redundant as such a decorator could have other kwargs that alter the behavior but do not define a whitelist.
However, can live with changing them and I already anticipated that people would require it. It's just that I like this version better so decided to go with this instead. (I actually had the shorter kwarg names initially, but I like how explicit the current names are.)

2. In most cases, there is a redirect channel and it's the same value for all of them (`#bot-commands`) . Furthermore, it's now impossible to have multiple redirect channels. Previously, it used the whitelisted channels as the redirect channels. How about this:

I don't like that and picking a single, dedicated redirect channel is intended. The main reason is that if we want to redirect members to use a command somewhere instead of in the current context, it should be bot-commands. While the command may technically whitelisted in other channels, that doesn't make them the right place to be redirected to just to perform a bot command as those other channels are not bot-command channels and may have conversations of their own going.
In short, sending users to another whitelisted channel just to perform a bot command will typically result in the member using that channel in an off-topic manner. And the bot reply even told them to go there.
As a side note, we were not actually using multiple redirect channels at the moment. Instead, we added a new kwarg, hidden_channels, just to prevent non-botcommands channel from showing up in the redirect list to prevent "misredirection" to a channel where a command is allowed but only in the context of conversations actually happening there; not for random bot commands.
The reason I did not make bot-commands the defacto default is because a command may only be whitelisted by role (meaning that a member without that role should also be unable to use it in bot-commands) and/or to allow for a different channel that we consider appropriate to divert bot command invocation to. (As this is only @Python, I can't think of a reason for it, but we could want to send users to seasonalbot-commands instead.)

   Furthermore, consider that a wrapper for the decorator could be created so that there is one decorator that has redirects disabled by default and the other which uses the channel whitelist.

We could have a "partial" that already has bot-commands as the redirect channel to avoid having it as a default (since that may cause surprises if we whitelist a context only by things like roles). I'm not entirely sure if it really adds anything. I like how explicit the current call is and we'd have to basically move that information into the decorator name to keep it explicit (in_whitelisted_context_with_bot_commands_redirect doesn't sound sexy, but I'm also unsure how of a shorter name that is as explicit.

I don't mind that a decorator spans over multiple lines,

A difference of opinion, but I think it's quite unsightly.

the kwarg names make explicit what each individual kwarg does and

Being explicit is good but it can become overly verbose, like here.

it's highly readable

In terms of understanding what it does, I agree. However, I think it's length/size due to its verbosity makes it difficult to read as it really catches one's attention, whereas one could be focusing on more important things like the function signature or body.

such a decorator could have other kwargs that alter the behavior but do not define a whitelist.

I don't think it should compromise (well, it's a compromise in my opinion anyway) to plan for potential future arguments. When those new arguments come, then we can reconsider if we cannot come up with names that can't be confused with the whitelist args; it's not really an expensive change to make down the road.

I think this looks much better (not as confident about renaming redirect_channels though):
@in_whitelist(
    channels=EVAL_CHANNELS,
    categories=EVAL_CATEGORIES,
    roles=EVAL_ROLES,
    redirect=Channels.bot_commands
)

And if we make the first 3 not kwarg-only:
@in_whitelist(EVAL_CHANNELS, EVAL_CATEGORIES, EVAL_ROLES, redirect=Channels.bot_commands)

Now even the longest of invocations fits on one line, and it's still readable to me. I don't need kwargs because the constant names already tell me what the types are. Maybe if it was 3 tuple literals instead it'd be harder to look at (i.e. to visually spot the boundaries of the tuples). However, I think it should be up to the programmer to decide whether or not the code is readable without explicit kwarg names.
A different direction to take this in would be separate decorators for roles, channels, and categories. Didn't think about that idea much but just want to throw it out there.

In short, sending users to another whitelisted channel just to perform a bot command will typically result in the member using that channel in an off-topic manner. And the bot reply even told them to go there.

I can see why from a common sense point of view, this is a bad design. I suggested it because, in practice, 4/6 uses of the decorator only have bot commands as the whitelisted channel. Given my distaste with the verbosity of the decorator, I'd want to address that common use case somehow to shorten the invocation. Maybe this is not the right solution after all, but we should at least look into doing something.

As a side note, we were not actually using multiple redirect channels at the moment.

True, but I do recall it being used in the past for some seasonalbot stuff. If you believe there'd never be a case where we need to have multiple redirect channels, then it's fine to restrict it to one.

The reason I did not make bot-commands the defacto default is because a command may only be whitelisted by role (meaning that a member without that role should also be unable to use it in bot-commands) and/or to allow for a different channel that we consider appropriate to divert bot command invocation to.

Well if it's either nothing or #bot-commands, then how about making it a boolean? I can't think of any other channel to redirect to either, but it would limit us down the road should the need arise.

However, I'm not really sure how we would name such a "partial" that is as explicit as having the redirect kwarg there but makes the use more convenient by being shorted.

I agree it is tricky to name. Giving it a long name would defeat the purpose. How about in_bot_commands? The intent would be to use it for commands that only need #bot-commands whitelisted, though technically other channels could be specified since the redirect is always implicitly whitelisted. However, I concede that if someone doesn't know bot_commands is a channel, then that name would be unclear. Furthermore, it'd be unclear it still allows role and category whitelists. I think the boolean idea for the redirect is more appealing.

And if we make the first 3 not kwarg-only:
@in_whitelist(EVAL_CHANNELS, EVAL_CATEGORIES, EVAL_ROLES, redirect=Channels.bot_commands)

Now even the longest of invocations fits on one line, and it's still readable to me. I don't need kwargs because the constant names already tell me what the types are. Maybe if it was 3 tuple literals instead it'd be harder to look at (i.e. to visually spot the boundaries of the tuples). However, I think it should be up to the programmer to decide whether or not the code is readable without explicit kwarg names.

I'm not a fan of the the proposed API allowing positional arguments, as it's very easy to get it wrong and, if you do, it will fail silently. I don't think allowing optional arguments with an identical type pattern as positional arguments is a good idea.
Take this alternative call, for example:
@in_whitelist(EVAL_CHANNELS, EVAL_ROLES, EVAL_CATEGORIES, redirect=Channels.bot_commands)

This looks entirely reasonable when you're reading it and the names of the constants give the impression that it's easy to understand which argument does what. In reality, the EVAL_ROLES and EVAL_CATEGORIES argument are accidentally swapped making it so that the container of role IDs will be used for the category check and the container with category IDs will be used for the role check.
This will fail silently, as they are both just containers of integers.
An other easy mistake is when we want to remove one of the three whitelists from the decorator; it's very easy to go something like this
@in_whitelist(EVAL_CHANNELS, EVAL_CATEGORIES, EVAL_ROLES, redirect=Channels.bot_commands)

to something like this:
@in_whitelist(EVAL_CHANNELS, EVAL_ROLES, redirect=Channels.bot_commands)

when you want to remove the category whitelist. It looks like it should work but in reality you've just shifted the roles whitelist to the category position and it will break silently again.
Obviously, we could shift the burden of detecting this to the code review process, but just as it's easy to get these wrong for the original contributors, it's also easy to read over it for a reviewer. As it's a "fail silent", the only other way to catch it is by thoroughly testing it and noticing the difference in behavior. However, while I would very much like for us to test reviews thoroughly, I don't think it's unlikely for someone to miss this in any review larger than just specifically adding the decorator.
All in all, I'm not a fan of the proposed API and I don't think the argument of saving a few lines is strong enough to dismiss these API considerations. I know that made multi-line decorator invocations are a common pet peeve, but I think the API is just a way more important factor than avoiding a decorator that may take up multiple lines if you use all the arguments at all costs.
(I'm obviously shortening the kwarg names by dropping the whitelisted prefix and I'll adapt the shorted in_whitelist name for the decorator as well. That will probably bring "line break relief" in a lot of the locations where it's used, as they don't pass all three whitelists.)

Okay, just to chime in here with some quick input:

Shortening the kwarg names to drop whitelisted prefix is nice.
in_whitelist is a good decorator name.
I agree with @SebastiaanZ about positional arguments being too error prone to be justified.

The reason I did not make bot-commands the defacto default is because a command may only be whitelisted by role (meaning that a member without that role should also be unable to use it in bot-commands) and/or to allow for a different channel that we consider appropriate to divert bot command invocation to. (As this is only @Python, I can't think of a reason for it, but we could want to send users to seasonalbot-commands instead.)

I'm not sure I buy this. Why would we use this decorator to check only roles? We already have with_role for that, do we not? It might even be worth considering whether to assert that at least one of the channel lists has been passed, to prevent people from using it as a role filter.
I think I agree with @MarkKoz here that if we're gonna be redirecting to #bot-commands nine out of ten times anyway, then it would be better to explicitely set redirect to None or something in the few cases where we don't want that behavior, and of course allow a different redirect channel for the cases where we'd want to redirect somewhere else, which might happen in regards to events.

We already have with_role for that, do we not?

Maybe we should not have.

So my suggestion is to default to #bot-commands, but to allow the user to pass either None for no redirect message (instead, some generic "you can't use this command" message would be shown), or some other channel ID to redirect to that instead.

Sounds good, that was the direction I was heading as well, so I agree.

Maybe we should not have. An in_whitelist(roles=ROLES) would do as well.

maybe. with_role arguably has better readability for this case, and it sounds like handling this well introduces a little more complexity to the decorator.. but I'm not violently opposed to generalizing around this decorator, I guess.

maybe. with_role arguably has better readability for this case, and it sounds like handling this well introduces a little more complexity to the decorator.. but I'm not violently opposed to generalizing around this decorator, I guess.

Why would it add complexity?
The only reason I currently see that would warrant additional complexity is if we'd want to raise a specific exception so that our error handler could handle the check failure in a different. However, in that case, we could define a specific decorator for that behavior. We're not currently doing that.