Use Reset() in default constructor of Memory<T>.

PR
Author
Editor
Reviewers
Assignment
Approval
Merge

#2358
@YohannDudouit
@tzanio
@camierjs + @pazner + @v-dobrev + @acfisher
06/26/21
⌛due 07/10/21
⌛due 07/17/21

This PR is now under review (see the table in the PR description). To help with the review process, please do not force push to the branch.

This change adds another difference in the Memory class behavior compared to a pointer -- it will be good to mention this in the description of the class -- unlike a pointer, it is initialized as empty (equivalent to setting a pointer to NULL) by the default ctor.
One drawback of this change is that some code will now perform redundant initialization, e.g. Vector::Vector(int s) in the case s > 0 -- it will first initialize data with data.Reset and then re-initialize with data.New. If we could conditionally call different Memory constructors, this would not have been a problem here, however we cannot.
Personally, I prefer to stick to the current simple pointer-like behavior, since this is mostly internal, low-level class that users will rarely, if ever, use directly -- user code should just use class Array instead. With this behavior we can fine-tune things (like the above ctor) and be more explicit about creating/initializing/(shallow)copying/overwriting/deleting Memory objects.
Obviously, I'm biased here, since I intentionally designed this class to be the way it is. I'll let other reviewers approve the PR if they like it -- I'll neither approve, nor reject it.

This change adds another difference in the Memory class behavior compared to a pointer -- it will be good to mention this in the description of the class -- unlike a pointer, it is initialized as empty (equivalent to setting a pointer to NULL) by the default ctor.

I can add this, though I don't think we should push too much this comparison with pointers as this class really does not work like a pointer.

One drawback of this change is that some code will now perform redundant initialization, e.g. Vector::Vector(int s) in the case s > 0 -- it will first initialize data with data.Reset and then re-initialize with data.New. If we could conditionally call different Memory constructors, this would not have been a problem here, however we cannot.

I'm planning to not call New but the constructor Memory::Memory(int) instead (which would avoid the call to Reset()), currently Memory(0) and New(0) are broken (we would expect New(0) to do relatively the same thing as Reset() but it doesn't). I started this in #2353 but I had some issues, so I favored a more atomic and incremental approach.
Currently, one has to write the following code to initialize a Memory object:
size > 0 ? data.New(size) : data.Reset();

This is really asking developers unnecessary efforts, do you really expect everyone that ever use a Memory object to be aware that New(0) should not be used, but Reset() instead? Handling the 0 case inside New is much cleaner IMHO:
template <typename T>
inline void Memory<T>::New(int size)
{
   if (size > 0)
   {
      capacity = size;
      flags = OWNS_HOST | VALID_HOST;
      h_mt = MemoryManager::GetHostMemoryType();
      h_ptr = (h_mt == MemoryType::HOST) ? NewHOST(size) :
              (T*)MemoryManager::New_(nullptr, size*sizeof(T), h_mt, flags);
   }
   else
   {
      Reset();
   }
}

Personally, I prefer to stick to the current simple pointer-like behavior, since this is mostly internal, low-level class that users will rarely, if ever, use directly -- user code should just use class Array instead. With this behavior we can fine-tune things (like the above ctor) and be more explicit about creating/initializing/(shallow)copying/overwriting/deleting Memory objects.

If you were the only one using this class that would probably be okay, but we're a team and it's difficult to use this class without creating bugs. I think anything that makes this class safer and cleaner is good to take, the less we have to worry about implicit rules not enforced by the code the more we can be productive and focus on writing applied math algorithms. I really don't understand why you don't want to make the code safer and cleaner when this has no performance impact.

Obviously, I'm biased here, since I intentionally designed this class to be the way it is. I'll let other reviewers approve the PR if they like it -- I'll neither approve, nor reject it.

If you have any example of code that would show any advantage of the current design over this one, please share with the team.

This change adds another difference in the Memory class behavior compared to a pointer -- it will be good to mention this in the description of the class -- unlike a pointer, it is initialized as empty (equivalent to setting a pointer to NULL) by the default ctor.

I think Veselin brings up some valid points, but I wanted to mention one (somewhat pedantic) thing: the default constructor for a pointer does set it to NULL:
double *p{}; // default constructor
p == nullptr; // true

Now admittedly, C++ is a little weird about this, since usually you don't need to add {} to call the default constructor, but that is a weirdness of the language: default initialization of class types calls the constructor, and default initialization of non-class types does not call the constructor.
Another example,
std::vector<double*> v(1); // one-element default-initialized vector
v[0] == nullptr; // true

So it is actually consistent with the behavior of a pointer to set it NULL in the default constructor. What is inconsistent is that a raw pointer is a non-class type, and Memory<T> is a class type, but that is not in our control.

Adding () after the name of a variable is called value-initialization (https://en.cppreference.com/w/cpp/language/value_initialization) which is different from default-initialization (https://en.cppreference.com/w/cpp/language/default_initialization). These are two different things, at least for simple types like int, double, and pointers.
Notably, STL containers invoke value-initialization when constructing new elements.

Adding () after the name of a variable is called value-initialization (https://en.cppreference.com/w/cpp/language/value_initialization) which is different from default-initialization (https://en.cppreference.com/w/cpp/language/default_initialization). These are two different things, at least for simple types like int, double, and pointers.
Notably, STL containers invoke value-initialization when constructing new elements.

Yes, I know. The point being that default initialization of non-class types does not call the default constructor. The default constructor of raw pointers sets the pointer to NULL.

The point being that default initialization of non-class types does not call the default constructor. The default constructor of raw pointers sets the pointer to NULL.

I'm not sure "default constructor" is actually defined for pointers -- neither int *p nor int *p() invoke a "default constructor". According to https://en.cppreference.com/w/cpp/language/value_initialization, the second form performs zero initialization which in turn is done by the rule: "If T is a scalar type, the object's initial value is the integral constant zero explicitly converted to T. " -- it does not mention "default constructor".
In any case, this distinction is irrelevant for class Memory since both forms invoke the default constructor:
      Memory<int> m1;
      Memory<int> m2{};
      Memory<int> m3();  // cannot use: "warning: interpreted as function declaration"

This is really asking developers unnecessary efforts, do you really expect everyone that ever use a Memory object to be aware that New(0) should not be used, but Reset() instead? Handling the 0 case inside New is much cleaner IMHO:

The choice in class Vector to set the pointer to NULL when size is 0, is specific choice for that class. The choice for Memory::New(0) to behave as it does is for consistency with a new expression -- when invoked with a size of 0, it returns a non-null pointer. This choice seems more appropriate to me for class Memory.

I'm not sure "default constructor" is actually defined for pointers -- neither int *p nor int *p() invoke a "default constructor". According to https://en.cppreference.com/w/cpp/language/value_initialization, the second form performs zero initialization which in turn is done by the rule: "If T is a scalar type, the object's initial value is the integral constant zero explicitly converted to T. " -- it does not mention "default constructor".

You're probably right, and "default constructor" may not be the precise terminology in the standard.
What I was trying to communicate in the above posts is the following:
double *p1; // implementation-defined value
double *p2{}; // nullptr

Memory<double> m1; // (mfem master) implementation-defined value
Memory<double> m2{}; // (mfem master) implementation-defined value

Memory<double> m1; // (this PR) nullptr
Memory<double> m2{}; // (this PR) nullptr

In master, Memory is inconsistent with a raw pointer for value initialization, and in this PR Memory is inconsistent with a raw pointer for default initialization. There is no way (as far as I know) to make Memory consistent in both cases, because a class type behaves differently from a non-class type. So, we have to pick which inconsistency we prefer.

@pazner:

So, we have to pick which inconsistency we prefer.

I agree, this specific choice is matter of preference -- ultimately both choices can work.
@YohannDudouit:

I really don't understand why you don't want to make the code safer and cleaner when this has no performance impact.

I prefer, for this low-level class, the current, more explicit design where all basic operation like initialization, deletion, etc are done explicitly similar to how one works with pointers. For this class, I think this design actually makes the code easier to understand and follow. The concern that this is bug-prone is not not a big concern for me since this is a low-level, internally used class.
I'm just expressing my opinion here. If the majority prefers to accept this PR, I'm perfectly fine with that.

I don't have a strong preference either way. On one hand this is a low-level concept and using the corresponding low-level abstraction (raw pointer) makes sense. On the other hand, Memory is a class, and so I can one can expect it to behave in a class-like way.
Maybe it will be best if we ask the folks that may work with this class more directly what they think -- @camierjs, @artv3, @tomstitt, @dylan-copeland -- do you have a preference one way or the other?

The arguments on both sides make sense, and I doubt it will make a big difference, but my personal preference is to be cautious and initialize things rather than relying on human understanding of the correct behavior, although that understanding may be necessary anyway. So I slightly prefer to use this PR.

As a user, I think this is a large improvement and I strongly support this getting merged. Uninitialized pointer variable errors are very hard to track down.

Merged in next for testing...


WIP: Towards abstract MARGO data structures

I've started addressing https://github.com/hdrake/ClimateMARGO.jl/issues/29 here.

Should see if the issue of JuMPification of the MARGO diagnostics (https://github.com/hdrake/ClimateMARGO.jl/issues/18) can be addressed in this PR. It really would be a pain if every diagnostic would have to be re-written within JuMP every time an existing one is changed or a new one is implemented.


Content import and exports limited to 10,000?

Customer indicated that exports and imports from content search screen seem to be limited to 10,000 - is this specific to dockerized version or an issue with 5.x?
Customer had a valid license file applied to the the server at the time.

That is a hard limit on returning contents.
https://github.com/dotCMS/core/blob/master/dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ESContentletAPIImpl.java#L226

That matches the fact that I was able to reproduce this issue without docker involved.


WebSocket Client Support

Closes #650
Depends on #894 #898 #900 #901 #902 #903

Codecov Report

Merging #933 (10b760f) into main (21cdee6) will increase coverage by 1.40%.
The diff coverage is 76.92%.

@@            Coverage Diff             @@
##             main     #933      +/-   ##
==========================================
+ Coverage   53.93%   55.34%   +1.40%
==========================================
  Files          73       71       -2
  Lines        2171     2219      +48
  Branches       59       64       +5
==========================================
+ Hits         1171     1228      +57
+ Misses       1000      991       -9

Impacted Files
Coverage Δ

...o-http/src/main/scala/zhttp/socket/SocketApp.scala
25.71% <0.00%> (-0.76%)
:arrow_down:

...main/scala/zhttp/service/WebSocketAppHandler.scala
28.00% <25.00%> (ø)

...p/src/main/scala/zhttp/socket/SocketProtocol.scala
25.71% <26.31%> (-0.38%)
:arrow_down:

zio-http/src/main/scala/zhttp/http/Scheme.scala
68.96% <66.66%> (+8.96%)
:arrow_up:

zio-http/src/main/scala/zhttp/http/URL.scala
87.50% <84.37%> (-10.07%)
:arrow_down:

...la/zhttp/service/client/ClientInboundHandler.scala
85.71% <90.90%> (+14.28%)
:arrow_up:

zio-http/src/main/scala/zhttp/service/Client.scala
82.35% <94.23%> (+27.45%)
:arrow_up:

...main/scala/zhttp/service/EncodeClientRequest.scala
100.00% <100.00%> (ø)

...io-http/src/main/scala/zhttp/service/package.scala
100.00% <100.00%> (ø)

...http/service/server/ServerChannelInitializer.scala
93.75% <100.00%> (ø)

... and 10 more

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 21cdee6...10b760f. Read the comment docs.


WIP: use html5lib 0.999999999

See #403

Coverage increased (+0.08%) to 78.684% when pulling ae70ecefcf5b94d21571e7865893c85f39dd53b4 on tschorr:use_current_html5lib into c27b65eadc50d86dcfbc64b1e76f8d3a6efca6d4 on divio:master.


Create Posts “20221013-python-simultaneous-assignment”

Automatically generated by Netlify CMS

👷 Deploy Preview for monetaboot processing.

Name
Link

🔨 Latest commit
19cfb7387a4c7be72d10617b2ffbde4cd530796f

🔍 Latest deploy log
https://app.netlify.com/sites/monetaboot/deploys/63478002fa7ad800082e379b


phase3: update weave CNI plugin to 2.4.0

1.9.4 is quite outdated and is causing errors for k8s 1.12.
Use weave 2.4.0 and also consolidate the JSON files into a single
List object.
The 2.4.0 manifest is based on:
curl -s "https://cloud.weave.works/k8s/net.json?k8s-version=v1.10.0" |
jq . > phase3/weave-net/weave-net-list.json
Also bump the kubectl version to 1.12.0-beta.1 as 1.6.x is ancient.
ref: https://github.com/kubernetes/kubernetes/issues/68475
notes:

this is against the kubeadm-e2e branch.
i have no means to tests this because phase1 fails for me on GCE with unrelated terraform errors that i can't figure out.

/kind bug
/assign @errordeveloper
/cc @timothysc @bboreham

/priority critical-urgent

@roberthbailey - PTAL this is critical atm.
/lgtm
/approve

i also need someone to double check the change in terms of jsonnet and kubeadm -f apply <folder>.
it should work, by just having a file with List object in that folder instead of the separate object files.

@errordeveloper

So what you want to add to the docs is that

can you elaborate what docs are you referring to?
kubeadm docs or k-a docs?
the k-a GCE docs already have a weave step:
https://github.com/kubernetes/kubernetes-anywhere/blob/ec6155b8a83aac93b6a3e82c830847ef64bc2ce9/phase1/gce/DEVEL.md#deploying-a-kubernetes-development-cluster
kubectl apply -f https://git.io/weave-kube

this gets 2.4.0.

@fabriziopandini found a failure with the new manifest:
+ kubectl apply -f clusters/kubernetes/.tmp
error: error validating "clusters/kubernetes/.tmp/weave-net-list.json": error validating data: [field spec.template.spec.containers[0].securityContext.procMount for v1.SecurityContext is required, field spec.template.spec.containers[1].securityContext.procMount for v1.SecurityContext is required]; if you choose to ignore these errors, turn validation off with --validate=false
Makefile:71: recipe for target 'addons' failed
make: *** [addons] Error 1

investigating.

now using the suggested line by @errordeveloper
curl -s "https://cloud.weave.works/k8s/net.json?k8s-version=v1.10.0" |
jq . > phase3/weave-net/weave-net-list.json

@fabriziopandini
verified that this manifest works, looks like the json conversation that i did might have messed something up. this one got the manifest as json directly.

updated this too:

Also bump the kubectl version to 1.12.0-beta.1 as 1.6.x is ancient.

Ping me if you need this merged before I have a chance to fix up the repository ACLs.

@errordeveloper we had a discussion about kubernetes-anywhere docs at the SIG cluster lifecycle meeting today and the consensus was that we shouldn't add any docs updates for this project.
the project is maintained for e2e only.
@fabriziopandini PTAL, both you and me now have merge access, thanks to Robert, but we can wait a couple of hours more if there are further comments here.


After installed from app store, can't found app in extensions menu

Same thing happening with me, can't find "XCode Source Editor" option under Extensions

Does it support Xcode 12? I am using Big Sur and QuickType is not showing as an extension. Please refer to this link. Any update on whether this will be fixed anytime soon?

Sorry, we are not actively working on this project. PRs are welcome to update this, though.


[Solvers] ConvOclDirectFwdGen: split ConvolutionContext and ProblemDescription (stage 2)

PR splits ConvolutionContext and ProblemDescription inside ConvOclDirectFwdGen
Blocked by https://github.com/ROCmSoftwarePlatform/MIOpen/pull/1694

@averinevg the [Solver] series of PRs have conflicts with recent FIN updates, let's work on resolving them first. Thanks!
CC:  @JehandadKhan  @cderb

@averinevg the [Solver] series of PRs have conflicts with recent FIN updates, let's work on resolving them first. Thanks! CC: @JehandadKhan @cderb

@junliume These PRs are still work in progress, and they must be merged into development branch after https://github.com/ROCmSoftwarePlatform/MIOpen/pull/1694

@averinevg the [Solver] series of PRs have conflicts with recent FIN updates, let's work on resolving them first. Thanks! CC: @JehandadKhan @cderb

@junliume These PRs are still work in progress, and they must be merged into development branch after #1694

Thanks @averinevg ! Unfortunately, #1694  has to be reverted due to conflicts with #1669 , can we re-open #1694 after fixing the conflicts?

@averinevg the [Solver] series of PRs have conflicts with recent FIN updates, let's work on resolving them first. Thanks! CC: @JehandadKhan @cderb

@junliume These PRs are still work in progress, and they must be merged into development branch after #1694

Thanks @averinevg ! Unfortunately, #1694 has to be reverted due to conflicts with #1669 , can we re-open #1694 after fixing the conflicts?

@junliume Yes, sure


Possible memory leak created by watchers

Vue version
a95554d
Link to minimal reproduction
https://sfc.vuejs.org/#eNqFkcFugzAMhl/FyqUgUbJdK1pp2mVPsFMuAdxBFZLICXBAvPsMlGnapPaEf/vzj/xnEm/e50OP4iSKUFHrIwSMvb8o23beUYQJCHUV2wEzGHWsGpjhSq6DA68dlFW2MjoE+HxfP5OyAAOc4UXZeal57myIoLm3OyUWx/tCkqY7UT4glF3/negMkhTOF5jmlAspod8mUGljsN65DSqf0oXcruZ7WUTsvNERWQEUzevlA41xMDoydSFZLws/kMjEltGx0z6/BWc5xfV+dR8EJU5bIkuP81q0Ek2MPpykDNdqyf4WckdfkqucehvbDnMM3bEkNwYkNlYi++UhuTkgHQltjYT0yPMP+s93seVXmsX8DUUcsN0=
Steps to reproduce

Start chrome
Open the linked SFC Playground
Open devtools > Memory
Invoke garbage collection (click on trash)
Create snapshot
In the created snapshot, filter for VClass objects

What is expected?

What is actually happening?

System Info
No response
Any additional comments?
No response

If you open JS tab in a playground you will see that a and b are returned from setup and available to use in render from $setup argument.

I can't reproduce this. Both a and b are found in snapshots.
like @miroljub1995 said, they should not be collected.


Fix check for is_enterprise_install for slash commands

Summary
Describe the goal of this PR. Mention any related Issue numbers.
Requirements (place an x in each [ ])

[X ] I've read and understood the Contributing Guidelines and have done my best effort to follow them.
[ X] I've read and agree to the Code of Conduct.

Not sure yet when or why this suddenly started failing in the past couple days, but this fixes an issue I ran into today where "is_enterprise_install" === "false", but still passes a truthiness check further down the code path.
I posted an issue originally here: https://github.com/slackapi/bolt-js/issues/737

 Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

 Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

Thanks for sending the PR! I'll take a look shortly. I'll see if I can reproduce the original error. But I don't see any issues with your change here

Thanks for sending the PR! I'll take a look shortly. I'll see if I can reproduce the original error. But I don't see any issues with your change here


Object with optional properties is not type narrowed

Bug Report

🔎 Search Terms

Type narrowing optional property
🕗 Version & Regression Information

This is the behavior in every version I tried, and I reviewed the FAQ for entries about type narrowing.

⏯ Playground Link

Playground link with relevant code
💻 Code
type Slot = { id?: string }

function needsId(slot: { id: string }) {
}

function test(slot: Slot) {
  if (slot.id) {
    needsId(slot)
  }
}

🙁 Actual behavior

The compiler complains that on the line where needsId is called, string | undefined cannot be assigned to string.
🙂 Expected behavior

Inside the if-statement, the Typescript compiler knows that slot.id is a string. I expect slot to be narrowed to Slot & { id: string } inside the if-statement, so that needsId(slot) is a valid call.

Quoting @jack-williams:

Type guards do not propagate type narrowings to parent objects. The narrowing is only applied upon access of the narrowed property which is why the destructing function works, but the reference function does not. Narrowing the parent would involve synthesizing new types which would be expensive. More detail in this comment.


Refactor ExtAuthz config as a global config that's only read once

As it stands, ExtAuthz config is read multiple times during a reconcile cycle, completely unnecessary seeing as it cannot be changed at runtime.
This refactors things to:

Move the config to the config package.
Load the config only once at startup.
Replace manual parsing with envconfig.

Note: The individual commits are meaningful and can be reviewed one-by-one.

/lgtm


Encoding related bug in LDA.

What changes were proposed in this pull request?
I have found a major bug, without fixing it people will not be able to use LDA, or perhaps other algorithms too.
I spotted the bug and made a small fix, I would like you to fix the rest.
What type of PR is it?
[Bug Fix]
What is the Jira issue?
???
How was this patch tested?
I intended to use LDA for my data, on EMR as usually, LDA failed to process my text. So I checked your test, and when added code to add my data to your test instead of your two lines. When it run successfully, I realized that the test may be faulty, and indeed, I think close() is called in real life, but not in the test. Same errors as on EMR showed up.
I found the lines in features that coursed the errors:
na‹ve:1
xž:1
Why I do not know, but that means that I have to pre-process data prior to testing LDA further, plus I start doubting whether it will work for other languages.
EMR error messages for different options for memory and number of reduces are below. Same source, same reason.
Diagnostic Messages for this Task:
Error: java.lang.RuntimeException: Hive Runtime Error while closing operators: Exception caused in the iterative training
at org.apache.hadoop.hive.ql.exec.mr.ExecReducer.close(ExecReducer.java:287)
at org.apache.hadoop.mapred.ReduceTask.runOldReducer(ReduceTask.java:454)
at org.apache.hadoop.mapred.ReduceTask.run(ReduceTask.java:393)
at org.apache.hadoop.mapred.YarnChild$2.run(YarnChild.java:164)
at java.security.AccessController.doPrivileged(Native Method)
at javax.security.auth.Subject.doAs(Subject.java:422)
at org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1698)
at org.apache.hadoop.mapred.YarnChild.main(YarnChild.java:158)
Caused by: org.apache.hadoop.hive.ql.metadata.HiveException: Exception caused in the iterative training
at hivemall.topicmodel.LDAUDTF.runIterativeTraining(LDAUDTF.java:511)
at hivemall.topicmodel.LDAUDTF.close(LDAUDTF.java:309)
at org.apache.hadoop.hive.ql.exec.UDTFOperator.closeOp(UDTFOperator.java:152)
at org.apache.hadoop.hive.ql.exec.Operator.close(Operator.java:683)
at org.apache.hadoop.hive.ql.exec.Operator.close(Operator.java:697)
at org.apache.hadoop.hive.ql.exec.Operator.close(Operator.java:697)
at org.apache.hadoop.hive.ql.exec.mr.ExecReducer.close(ExecReducer.java:279)
... 7 more
Caused by: java.lang.OutOfMemoryError: Java heap space
at hivemall.topicmodel.LDAUDTF.runIterativeTraining(LDAUDTF.java:352)
... 13 more
Error: java.lang.RuntimeException: Hive Runtime Error while closing operators: Exception caused in the iterative training
at org.apache.hadoop.hive.ql.exec.mr.ExecReducer.close(ExecReducer.java:287)
at org.apache.hadoop.mapred.ReduceTask.runOldReducer(ReduceTask.java:454)
at org.apache.hadoop.mapred.ReduceTask.run(ReduceTask.java:393)
at org.apache.hadoop.mapred.YarnChild$2.run(YarnChild.java:164)
at java.security.AccessController.doPrivileged(Native Method)
at javax.security.auth.Subject.doAs(Subject.java:422)
at org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1698)
at org.apache.hadoop.mapred.YarnChild.main(YarnChild.java:158)
Caused by: org.apache.hadoop.hive.ql.metadata.HiveException: Exception caused in the iterative training
at hivemall.topicmodel.LDAUDTF.runIterativeTraining(LDAUDTF.java:511)
at hivemall.topicmodel.LDAUDTF.close(LDAUDTF.java:309)
at org.apache.hadoop.hive.ql.exec.UDTFOperator.closeOp(UDTFOperator.java:152)
at org.apache.hadoop.hive.ql.exec.Operator.close(Operator.java:683)
at org.apache.hadoop.hive.ql.exec.Operator.close(Operator.java:697)
at org.apache.hadoop.hive.ql.exec.Operator.close(Operator.java:697)
at org.apache.hadoop.hive.ql.exec.mr.ExecReducer.close(ExecReducer.java:279)
... 7 more
Caused by: java.nio.BufferUnderflowException
at java.nio.DirectByteBuffer.get(DirectByteBuffer.java:271)
at java.nio.ByteBuffer.get(ByteBuffer.java:715)
at hivemall.topicmodel.LDAUDTF.runIterativeTraining(LDAUDTF.java:356)
... 13 more
Error: java.lang.RuntimeException: Hive Runtime Error while closing operators: Exception caused in the iterative training
at org.apache.hadoop.hive.ql.exec.mr.ExecReducer.close(ExecReducer.java:287)
at org.apache.hadoop.mapred.ReduceTask.runOldReducer(ReduceTask.java:454)
at org.apache.hadoop.mapred.ReduceTask.run(ReduceTask.java:393)
at org.apache.hadoop.mapred.YarnChild$2.run(YarnChild.java:164)
at java.security.AccessController.doPrivileged(Native Method)
at javax.security.auth.Subject.doAs(Subject.java:422)
at org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1698)
at org.apache.hadoop.mapred.YarnChild.main(YarnChild.java:158)
Caused by: org.apache.hadoop.hive.ql.metadata.HiveException: Exception caused in the iterative training
at hivemall.topicmodel.LDAUDTF.runIterativeTraining(LDAUDTF.java:511)
at hivemall.topicmodel.LDAUDTF.close(LDAUDTF.java:309)
at org.apache.hadoop.hive.ql.exec.UDTFOperator.closeOp(UDTFOperator.java:152)
at org.apache.hadoop.hive.ql.exec.Operator.close(Operator.java:683)
at org.apache.hadoop.hive.ql.exec.Operator.close(Operator.java:697)
at org.apache.hadoop.hive.ql.exec.Operator.close(Operator.java:697)
at org.apache.hadoop.hive.ql.exec.mr.ExecReducer.close(ExecReducer.java:279)
... 7 more
Caused by: java.lang.NegativeArraySizeException
at hivemall.topicmodel.LDAUDTF.runIterativeTraining(LDAUDTF.java:352)
... 13 more
How to use this feature?
When fixed people will be able to run LDA.

@helenahm thank you for the bug report.
@takuti
String.getBytes() and new String(byte[]) should be avoided to avoid encoding/decoding problem (UTF16 to . See the JDK implementation.
Better to replace the current code using String.charAt(int) and new String(char[]). See hivemall.utils.IOUtils.
        final int len = s.length();
        out.writeInt(len);
        for (int i = 0; i < len; i++) {
            int v = s.charAt(i);
            out.writeChar(v);
        }

        final char[] ch = new char[len];
        for (int i = 0; i < len; i++) {
            ch[i] = in.readChar();
        }
        return new String(ch);

@helenahm Good catch, and thank you for reporting. You are right; this bug is definitely caused by the encoding-related issue. I'll take care of fixing the bug.
Basically, the critical reason is in:
buf.putInt(wc.length());
buf.put(wc.getBytes());

These lines are totally wrong and need be fixed to:
buf.putInt(wc.getBytes().length);
buf.put(wc.getBytes());

At the same time, as @myui mentioned above, using bytes things should be avoided.

Thank you for the library you are developing. Please, let me know when the code is ready to be pulled to build a new hivemall-core jar and use it for LDA.

Also, in my query (below), I had to add ln != null, to not submit empty text, otherwise an error (below below) is thrown
create temporary function train_lda as 'hivemall.topicmodel.LDAUDTF';
with word_counts as (
select
cast(rownum as int) as docid,
feature(word, count(word)) as word_count
from data_rownum t1 LATERAL VIEW explode(tokenize(ln, true)) t2 as word
where
not is_stopword(word) and ln != ""
group by
rownum, word
)
select
label, word, avg(lambda) as lambda
from (
select
train_lda(feature, "-topics 2 -iter 20") as (label, word, lambda)
from (
select docid, collect_set(word_count) as feature
from word_counts
group by docid
) t1
) t2
group by label, word
order by lambda desc
;
Diagnostic Messages for this Task:
Error: java.lang.RuntimeException: org.apache.hadoop.hive.ql.metadata.HiveException: Hive Runtime Error while processing row {"rownum":"179","ln":null}
at org.apache.hadoop.hive.ql.exec.mr.ExecMapper.map(ExecMapper.java:169)
at org.apache.hadoop.mapred.MapRunner.run(MapRunner.java:54)
at org.apache.hadoop.mapred.MapTask.runOldMapper(MapTask.java:455)
at org.apache.hadoop.mapred.MapTask.run(MapTask.java:344)
at org.apache.hadoop.mapred.YarnChild$2.run(YarnChild.java:164)
at java.security.AccessController.doPrivileged(Native Method)
at javax.security.auth.Subject.doAs(Subject.java:422)
at org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1698)
at org.apache.hadoop.mapred.YarnChild.main(YarnChild.java:158)
Caused by: org.apache.hadoop.hive.ql.metadata.HiveException: Hive Runtime Error while processing row {"rownum":"179","ln":null}
at org.apache.hadoop.hive.ql.exec.MapOperator.process(MapOperator.java:499)
at org.apache.hadoop.hive.ql.exec.mr.ExecMapper.map(ExecMapper.java:160)
... 8 more
Caused by: org.apache.hadoop.hive.ql.metadata.HiveException: Unable to execute method public java.util.List hivemall.tools.text.TokenizeUDF.evaluate(org.apache.hadoop.io.Text,boolean)  on object hivemall.tools.text.TokenizeUDF@6240651f of class hivemall.tools.text.TokenizeUDF with arguments {null, true:java.lang.Boolean} of size 2
at org.apache.hadoop.hive.ql.exec.FunctionRegistry.invoke(FunctionRegistry.java:991)
at org.apache.hadoop.hive.ql.udf.generic.GenericUDFBridge.evaluate(GenericUDFBridge.java:194)
at org.apache.hadoop.hive.ql.exec.ExprNodeGenericFuncEvaluator._evaluate(ExprNodeGenericFuncEvaluator.java:186)
at org.apache.hadoop.hive.ql.exec.ExprNodeEvaluator.evaluate(ExprNodeEvaluator.java:77)
at org.apache.hadoop.hive.ql.exec.ExprNodeEvaluator.evaluate(ExprNodeEvaluator.java:65)
at org.apache.hadoop.hive.ql.exec.SelectOperator.process(SelectOperator.java:88)
at org.apache.hadoop.hive.ql.exec.Operator.forward(Operator.java:879)
at org.apache.hadoop.hive.ql.exec.LateralViewForwardOperator.process(LateralViewForwardOperator.java:39)
at org.apache.hadoop.hive.ql.exec.Operator.forward(Operator.java:879)
at org.apache.hadoop.hive.ql.exec.FilterOperator.process(FilterOperator.java:126)
at org.apache.hadoop.hive.ql.exec.Operator.forward(Operator.java:879)
at org.apache.hadoop.hive.ql.exec.TableScanOperator.process(TableScanOperator.java:130)
at org.apache.hadoop.hive.ql.exec.MapOperator$MapOpCtx.forward(MapOperator.java:149)
at org.apache.hadoop.hive.ql.exec.MapOperator.process(MapOperator.java:489)
... 9 more
Caused by: java.lang.reflect.InvocationTargetException
at sun.reflect.GeneratedMethodAccessor4.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.apache.hadoop.hive.ql.exec.FunctionRegistry.invoke(FunctionRegistry.java:967)
... 22 more
Caused by: java.lang.NullPointerException
at hivemall.tools.text.TokenizeUDF.evaluate(TokenizeUDF.java:42)
... 26 more

@helenahm If I understand correctly, it sounds natural. Make sure the difference between "null" and "empty" document. Null document must not exist in your document collection (or you manually need to add the AND ln != null clause for workaround as you already tried; i.e. carefully undergo preprocessing step).
In fact, since doc#3 is null, the following query throws an exception which you wrote:
with docs as (
  select docid, doc
  from (
    select 1 as docid, "Fruits and vegetables are healthy na‹ve." as doc
    union all
    select 2 as docid, "I like apples, oranges, and avocados. I do not like the flu or colds." as doc
    union all
    select 3 as docid, null as doc
  ) t1
),
word_counts as (
  select
    docid,
    feature(word, count(word)) as f
  from docs t1 LATERAL VIEW explode(tokenize(doc, true)) t2 as word
  where
    not is_stopword(word)
  group by
    docid, word
)
select label, word, avg(lambda) as lambda
from (
  select
    -- train_plsa(feature, "-topics 2 -eps 0.00001 -iter 2048 -alpha 0.01") as (label, word, lambda)
    train_lda(feature, "-topics 2 -iter 20") as (label, word, lambda)
  from (
    select docid, collect_set(f) as feature
    from word_counts
    group by docid
    -- order by docid
  ) t1
) t2
group by label, word
order by lambda desc
;

However, if the document is just empty, it works:
with docs as (
  select docid, doc
  from (
    select 1 as docid, "Fruits and vegetables are healthy na‹ve." as doc
    union all
    select 2 as docid, "I like apples, oranges, and avocados. I do not like the flu or colds." as doc
    union all
    select 3 as docid, "" as doc
  ) t1
),
...

@takuti It's another issue. tokenize should return null where input is null.
https://github.com/apache/incubator-hivemall/blob/master/core/src/main/java/hivemall/tools/text/TokenizeUDF.java#L42

@myui IMO, in that case, throwing well-described HiveException looks more reasonable because input and output type of tokenize UDF is dfifferent. Or, returning empty array is also acceptable.

@takuti Root cause is NPE. The RuntimeException is caught and re-thrown in https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java#L1039
The exception message is generated in Hive runtime. So, we cannot change it.
Root cause is that UDFs should not throw RuntimeException.
Caused by: java.lang.NullPointerException
at hivemall.tools.text.TokenizeUDF.evaluate(TokenizeUDF.java:42)
... 26 more

@myui ah, I see.
@helenahm I'm addressing the tokenize(null) issue, plz wait :)

i am pretty sure that in my case the document is empty, and not null, first of all because Ln != "" saved that query, not Ln is not null; also it is a table obtained as an external table, so if anything nulls would look like "null", that is a non empty string.
It is up to you, but as for the user of hivemall, LDA is at the end of a much longer pipeline, and nulls and empty strings will not be uncommon, your users may be upset to have such an error.
Also, as a user, after building a model and running predict, I would feel good if null and empty strings would get a topic, -1, or "none", or "Na". I do not know how it works now, but I think processing nulls and empty strings would make people feel better about the hivemall library.

You are right though, my bad, the error comes from null rows only. Sorry.

@helenahm No worries, and I have fixed both train_lda() and tokenize() in #83.
If the PR is merged:

you can use multi-byte strings e.g. xž and na‹ve
NULL document is allowed (and has no negative effect to the later topic modeling)

Could you check if the update works fine for you? The branch is fix-topicmodel in takuti/incubator-hivemall; you can build the fixed version by cloning the repository and checking out to the branch.

Yes, thank you very very much, the compiled code works without any errors! :-)

@helenahm Cool! Thank you for your cooperation. Could you close this PR if everything is okay?

@helenahm @takuti close this PR as resolved.


nixos/shell: added environment.shellFiles option

Motivation for this change
This will allow to use standard packages to add to the init shell scripts whitout having to go lookinto said package to fing the specific file to include.However I still need https://github.com/NixOS/nixpkgs/pull/82743 to not make the module system choke on the all the attributes normal packages have.
The edit on thefuck,while still improving the shell load times,is more to check that the system is working.
Things done

[x] Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
Built on platform(s)

[x] NixOS
[ ] macOS
[ ] other Linux distributions

[ ] Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
[ ] Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
[x] Tested execution of all binary files (usually in ./result/bin/)
[ ] Determined the impact on package closure size (by running nix path-info -S before and after)
[ ] Ensured that relevant documentation is up to date
[ ] Fits CONTRIBUTING.md.

Update:it turns out that you only need _modules.check=false to use packages as submodules.

I've modified a few packages to support the option,to both give an example and to give packages ready to be used

While there are many packages that still need to be upgraded to this option,I still think this is a good start

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:
https://discourse.nixos.org/t/prs-ready-for-review/3032/206

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:
https://discourse.nixos.org/t/prs-ready-for-review/3032/331

Upended by https://github.com/NixOS/nixpkgs/pull/110762

Upended by https://github.com/NixOS/nixpkgs/pull/110762


Check available replicas for deployment readiness

https://issues.redhat.com/browse/RHCLOUD-18222
Adding checks to a deployment status for available and updated replicas.

/retest


[EN] Feedback for: /web/fundamentals/getting-started/push-notifications/?hl=en

code doesnt work for me
gives me a registration id but I can't send to it using my api key
just says InvalidRegistration error.
-James

key is truncated with '...' in console.
I had to show registration id in a span.


Using sdba.processing.to_additive_space for precipitation with units of mm/day

Setup Information

Xclim version: 0.44.0
Xarray version: 2023.8.0
Python version: 3.11.5
Operating System: CentOS Linux release 7.3.1611

Description
I'm trying to use the sdba.processing.to_additive_space with precipitation data (units 'mm/day') and a lower bound of 0., but I'm getting this error:
DimensionalityError: Cannot convert from 'degree_Celsius' ([temperature]) to 'millimeter / day' ([length] / [time])
I believe it is being raised when attempting to convert the lower bound units. I have tried the same with temperature (units 'K') and it works fine. If I change the units of the precipitation data to 'K' it will run, but returns NaNs (I assume because of some unit conversion behind the scenes).
I'm using a DataArray with time series data and only one grid cell, the time coordinate is np.datetime64.
Steps To Reproduce
import xarray as xr
from xclim import sdba
from pathlib import Path
fpath = Path(f"/nesi/project/niwa00018/campbelli/statistical-downscaling/data/vcsn/VCSN_daily_pr5k_1972-2020.nc")
dset = xr.open_dataset(fpath).squeeze()["pr"].sel(latitude=-41.322, longitude=174.804, method="nearest")
dset.attrs.update({"units": "mm/day"})
sdba.processing.to_additive_space(dset, lower_bound=0)
Traceback
/home/campbelli/.conda/envs/bcsd/lib/python3.11/site-packages/xclim/core/formatting.py:397: FutureWarning: Future versions of xclim will require explicit unit specifications.
outs = func(*args, **kwargs)
Traceback (most recent call last):
File "", line 1, in
File "<boltons.funcutils.FunctionBuilder-264>", line 2, in to_additive_space
File "/home/campbelli/.conda/envs/bcsd/lib/python3.11/site-packages/xclim/core/formatting.py", line 397, in _call_and_add_history
outs = func(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^
File "/home/campbelli/.conda/envs/bcsd/lib/python3.11/site-packages/xclim/sdba/processing.py", line 678, in to_additive_space
lower_bound = convert_units_to(lower_bound, data)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/campbelli/.conda/envs/bcsd/lib/python3.11/site-packages/xclim/core/units.py", line 424, in convert_units_to
return units.Quantity(source, units=source_unit).to(target_unit).m
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/campbelli/.conda/envs/bcsd/lib/python3.11/site-packages/pint/facets/plain/quantity.py", line 517, in to
magnitude = self._convert_magnitude_not_inplace(other, *contexts, **ctx_kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/campbelli/.conda/envs/bcsd/lib/python3.11/site-packages/pint/facets/plain/quantity.py", line 462, in _convert_magnitude_not_inplace
return self._REGISTRY.convert(self._magnitude, self._units, other)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/campbelli/.conda/envs/bcsd/lib/python3.11/site-packages/pint/facets/plain/registry.py", line 961, in convert
return self._convert(value, src, dst, inplace)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/campbelli/.conda/envs/bcsd/lib/python3.11/site-packages/pint/facets/context/registry.py", line 403, in _convert
return super()._convert(value, src, dst, inplace)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/campbelli/.conda/envs/bcsd/lib/python3.11/site-packages/pint/facets/nonmultiplicative/registry.py", line 262, in _convert
raise DimensionalityError(src, dst, src_dim, dst_dim)
pint.errors.DimensionalityError: Cannot convert from 'degree_Celsius' ([temperature]) to 'millimeter / day' ([length] / [time])
Additional context
No response
Contribution

[X] I would be willing/able to open a Pull Request to address this bug.

Code of Conduct

[X] I agree to follow this project's Code of Conduct

Hi @isaachcamp,
Thanks for the details. Indeed, the problem stems from unit conversion failing. Specifically, to_additive_space expects lower_bound as a str which contains the units :

lower_bound :  str
The smallest physical value of the variable, excluded, as a Quantity
The data should only have values strictly larger than this bound.

so, you would need to call your function like this:
sdba.processing.to_additive_space(dset, lower_bound="0 mm/d")

and things will work out as expected. By the way, if you have other questions surrounding this topic, there's a section about this in this tutorial notebook.
Changes to make in xclim

Here,  convert_units_to(lower_bound, data) fails. There are many checks on the type of lower_bound (or source). In your case, your call with lower_bound=0 should have been caught by :

if isinstance(source, (float, int)):
    raise TypeError("Please specify units explicitly.")

but it seems to have evaded it. Defining the function in a notebook, I do get the expected TypeError:
xclim.core.units.convert_units_to(0, "mm/d") # I get the "Cannot convert from 'degree_Celsius'
convert_units_to_defined_in_a_jupyter(0, "mm/d")  # I get the expected TypeError message

I will investigate. Also, maybe the TypeError should be more explicit:  Saying explicitly that we want a Quantified or str would be good.

Your observation about temperatures is also interesting. In its simplest form:

from xclim.core.units import convert_units_to
convert_units_to(0, "mm/d") # this fails
convert_units_to(0, "K") # this succeeds

This will also probably be solved the first point above, when we fix the failing/succeeding conditions leading to the TypeError above, but I will be careful with this point. Perhaps this can have other implications elsewhere.

@isaachcamp  Sorry for this issue, the confusion comes from a legacy code block that we had forgotten in the unit conversion function. Up to version 0.44, it would guess temperature unit whenever the function received a number. At the very beginning of xclim this idea was kinda useful in some cases, but in the latter versions we began writing functions like to_additive_space where it simply makes no sense. But we forgot the code block there. It was removed very recently and a proper error should be raised with xclim 0.45.  (That might be the source of your confusion @coxipi)
Indeed, the better way to pass any kind of quantity with a magnitude and units, is to pass a string. You should be able to also pass a pint.Quantity object, but that has been less tested.

@aulemahal  Ah got it. I simply "mamba installed" xclim in a new environment, thought it would 0.45,  didn't realize it was 0.44.X version. The successful code I tested in a notebook was from 0.45.X.
I had checked CHANGES.rst and didn't see changes to the function,  I should learn how to compare code versions in GitHub, it would be more efficient haha!

Hi @coxipi and @aulemahal, thanks for being so quick to respond!
That works using lower_bound="0 mm/d", and just to check I updated to v0.45 and I get the TypeError. Making the error message more explicit could be good. You could perhaps add that it's the lower_bound argument that requires explicit units and provide an example, like "e.g., '0 mm/d'", but as it is in v0.45 I would have rechecked the docs and found the problem, so it's not essential. I wonder though if the units might be retrieved from the DataArray attributes, so they don't need to be specified for the lower bound?
Anyway, thanks for the help!


Rancher Compose: Isolation key is not set in the service api

**Rancher Version:**v1.2.0-pre4-rc7
**Docker Version:**1.12.3
**OS and where are the hosts located? (cloud, bare metal, etc):**Ubuntu, Cloud
**Setup Details: (single node rancher vs. HA rancher, internal DB vs. external DB)**Single node
**Environment Type: (Cattle/Kubernetes/Swarm/Mesos)**Cattle
Steps to Reproduce:

Create a stack with "isolation" key set using the yml file below:

version: '2'
services:
  test554094:
    device_write_bps:
      /dev/null: 200
    device_write_iops:
      /dev/vda: 3000
    cpu_quota: 20000
    read_only: true
    memswap_limit:<PHONE_NUMBER>
    oom_score_adj: 100
    cpu_shares: 400
    shm_size: 1024
    cgroup_parent: abcd
    mem_limit:<PHONE_NUMBER>
    security_opt:
    - label:user:USER
    - label:role:ROLE
    tmpfs:
    - /run
    device_read_bps:
      /dev/null: 4000
    group_add:
    - root
    blkio_weight: 1000
    stop_signal: SIGTERM
    dns_opt:
    - abc
    extra_hosts:
    - h1:<IP_ADDRESS>
    - h2:<IP_ADDRESS>
    cpu_period: 10000
    cpuset: '0'
    image: ibuildthecloud/helloworld:latest
    mem_reservation: 5000000
    device_read_iops:
      /dev/vda: 2000
    dns_search:
    - abc
    mem_swappiness: 100
    oom_kill_disable: true
    ipc: host
    isolation: default

Results
isolation key is not set in service api
The exported yml file does not have the isolation key:
version: '2'
services:
  test554094:
    device_write_bps:
      /dev/null: 200
    device_write_iops:
      /dev/vda: 3000
    cpu_quota: 20000
    read_only: true
    memswap_limit:<PHONE_NUMBER>
    oom_score_adj: 100
    cpu_shares: 400
    shm_size: 1024
    cgroup_parent: abcd
    mem_limit:<PHONE_NUMBER>
    security_opt:
    - label:user:USER
    - label:role:ROLE
    tmpfs:
    - /run
    device_read_bps:
      /dev/null: 4000
    group_add:
    - root
    blkio_weight: 1000
    stop_signal: SIGTERM
    dns_opt:
    - abc
    extra_hosts:
    - h1:<IP_ADDRESS>
    - h2:<IP_ADDRESS>
    cpu_period: 10000
    cpuset: '0'
    image: ibuildthecloud/helloworld:latest
    mem_reservation: 5000000
    device_read_iops:
      /dev/vda: 2000
    dns_search:
    - abc
    mem_swappiness: 100
    oom_kill_disable: true
    ipc: host

@StrongMonkey Isolation is being dropped in the /scripts/transform API.

@alena1108 Could you please take a look? I have test cases that already covered isolation, but i doubt this is beyond the logic i wrote in cattle. Is there any java classed that does this transformation?

I think ServiceDiscoveryConfigItem.java is only responsible for transforming launch_config into yml files.

@StrongMonkey this is the class responsible for fields transformation on the instance (and launch config):
./code/implementation/docker/api/src/main/java/io/cattle/platform/docker/api/transform/TransformInspect.java
And when rancher-compose invokes /scripts/transform API path, this is the cattle code that is being hit. And by some reason, "isolation" field is being stripped.
Try debugging by putting a break point here to figure out if the field is even coming with the request. If yes, and then it is being dropped, check how this field is any different from the rest of the fields you've added.

Will test with windows OS in 1.3.0

windowsOS, like macOS but with more powershell :)

lol

Tested with rancher-server version - v1.3.0-rc1
When a stack is created with the following yml file , service that gets created has isolation saved at the api level.
version: '2'
services:
  testhyper:
    image: microsoft/iis
    stdin_open: true
    network_mode: nat
    tty: true
    isolation: hyperv
    ports:
    - 9081:80/tcp
    labels:
      io.rancher.container.pull_image: always

Exported config of this service also has " isolation: hyperv" set in it.


apply,plugin这2个api在webpack5中被移除了，请问有对应的替换api吗

apply,plugin这2个api在webpack5中被移除了，请问有对应的替换api吗

It looks like you just deleted our lovely crafted issue template. It was there for good reasons. Please help us solving your issue by answering the questions asked in this template. I'm closing this. Please either update the issue with the template and reopen, or open a new issue.


MN-651: reset api-exported.yaml to v1.1.1

- What I did
- How I did it
- How to verify it
- Description for the changelog

Codecov Report

Merging #48 into master will increase coverage by 0.59%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #48      +/-   ##
==========================================
+ Coverage   15.30%   15.90%   +0.59%
==========================================
  Files           7        7
  Lines        1006     1006
==========================================
+ Hits          154      160       +6
+ Misses        844      833      -11
- Partials        8       13       +5

Impacted Files
Coverage Δ

...et/application/builtin/contract/deposit/deposit.go
2.64% <0.00%> (+2.64%)
: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 bdc85f6...064ceae. Read the comment docs.


Domain access from sandbox - bom.gov.au

Cannot seem to access bom.gov.au from Sandbox, though it should already be on the whitelist.
Sandbox:
jovyan@jupyter-alger:~/$ wget bom.gov.au
--2021-03-23 02:30:38--  http://bom.gov.au/
Resolving bom.gov.au (bom.gov.au)... <IP_ADDRESS>, <IP_ADDRESS>
Connecting to bom.gov.au (bom.gov.au)|<IP_ADDRESS>|:80... connected.
HTTP request sent, awaiting response... 301 Moved Permanently
Location: http://www.bom.gov.au/ [following]
--2021-03-23 02:30:38--  http://www.bom.gov.au/
Resolving www.bom.gov.au (www.bom.gov.au)... <IP_ADDRESS>
Connecting to www.bom.gov.au (www.bom.gov.au)|<IP_ADDRESS>|:80... connected.
HTTP request sent, awaiting response... 403 Forbidden
2021-03-23 02:30:38 ERROR 403: Forbidden.

DEV Sandbox:
jovyan@jupyter-alger:~/$ wget bom.gov.au
--2021-03-23 02:31:44--  http://bom.gov.au/
Resolving bom.gov.au (bom.gov.au)... <IP_ADDRESS>, <IP_ADDRESS>
Connecting to bom.gov.au (bom.gov.au)|<IP_ADDRESS>|:80... connected.
HTTP request sent, awaiting response... 301 Moved Permanently
Location: http://www.bom.gov.au/ [following]
--2021-03-23 02:31:48--  http://www.bom.gov.au/
Resolving www.bom.gov.au (www.bom.gov.au)... <IP_ADDRESS>
Connecting to www.bom.gov.au (www.bom.gov.au)|<IP_ADDRESS>|:80... connected.
HTTP request sent, awaiting response... 200 OK
Length: 38987 (38K) [text/html]
Saving to: ‘index.html’

index.html                                         100%[==============================================================================================================>]  38.07K  --.-KB/s    in 0.001s

2021-03-23 02:31:48 (41.2 MB/s) - ‘index.html’ saved [38987/38987]

I have also notified BOM in case it's not us.

BOM now blocks automated access to their datasets, so we are unable to download them automatically via the Sandbox.


Still active?

Is this project still active?  I would like to incorporate some additional functionality to the project, but if it is no longer maintained I will move forward with maintaining my own version.

It is, though we haven’t had much need making any changes in a while 🙂

Awesome!


Searching for SteamID doesn't include some log items

When searching the server log by Steam ID it doesn't show some event types, such as ADMIN PERMA BANNED. (I hid the players name with "playername")
Granted these might not be direct rcon logs, but they're helpful to be logged for search the logs versus going to the audit or trying to locate the player via the player search etc.
Fri 23rd Feb 08:38:40 | ADMIN PERMA BANNED | BAN: [playername] has been banned. [PERMANENTLY BANNED BY THE ADMINISTRATOR! VAC/Game ban history (114 days ago)] | playername |   | 1
-- | -- | -- | -- | -- | --

I noticed there is a Name 2 column, shouldn't all log messages have the steamid attached these such actions? As you can change your steam name.
Event Types
Includes Steam ID

KILL = SteamID in content column
MESSAGE = SteamID in content column

Doesn't Include Steam ID

TEAMSWITCH = Only Player Name
ADMIN BANNED = Only Player Name for both Kick and Ban
ADMIN MISC = Only Player Name.

Not every log event comes out of the game server with a player ID attached, some of them only include the player name.
We certainly could make an effort to try to look up player IDs during the parsing, but I'm concerned there would be a lot of weird ways we could fail or do it wrong and start recording incorrect player IDs.
I did ask Abu to update our RCON wishlist that we've provided T17 to include the player IDs in all log types though, that is going to be the ultimate best solution but T17 neglects RCON unfortunately.


Added a diagram to the incredibuild

Added a diagram to the incredibuild scaling on Azure doc

Added a diagram to the incredibuild


Restraint indices out of bounds in Trivalent Nitrogen Set 3

Describe the bug
I am trying to access the atoms involved in the optimizations in the "OpenFF Trivalent Nitrogen Set 3" dataset. I am able to access the indices, and I am able to access the molecule, but I find that the indices are out of bounds; i.e. the molecule seems to be lacking atoms. For instance, one of the molecules might have restraint indices [0, 13, 2, 1] but the molecule would only have 9 atoms, whereas it would need at least 14 here (due to zero-based indexing).
I have tried converting the SMILES for the molecule to Openey's oechem.OEMol and accessing the atoms in the resulting molecule, but the indices do not seem to match up with those in the qcelemental molecule. I think the atom at the first restraint index should be the trivalent nitrogen, but this is not always the case when using the OEMol.
To Reproduce
The following code prints out the molecules that I have been having issues with.
import numpy as np
import qcportal as ptl
from openeye import oechem, oedepict

# Loading Data
client = ptl.FractalClient()
ds = client.get_collection('GridOptimizationDataset',
                           "OpenFF Trivalent Nitrogen Set 3")
molecule_ids = [ds.get_entry(index).initial_molecule for index in ds.df.index]
molecules = client.query_molecules(molecule_ids)
smiles_strings = [
    ds.get_entry(index).attributes["canonical_explicit_hydrogen_smiles"]
    for index in ds.df.index
]
grid_opt_record_ids = [
    ds.get_entry(index).object_map['default'] for index in ds.df.index
]
grid_opt_records = client.query_procedures(grid_opt_record_ids)

# Looking at the restraint indices in each record, and seeing whether they are
# available in the molecule
for record, molecule, smiles in zip(grid_opt_records, molecules,
                                    smiles_strings):
    restraint_indices = np.array(record.keywords.scans[0].__dict__['indices'])
    num_atoms = len(molecule.symbols)

    # If any restraint index is out of bounds
    if np.any(restraint_indices >= num_atoms):
        print("--------------------------------------")
        print("GridOptRecord id :", record.id)
        print("SMILES           :", smiles)
        print("Restraint indices:", restraint_indices)
        print("Number of atoms  :", num_atoms)

        mol = oechem.OEMol()
        oechem.OESmilesToMol(mol, smiles)
        idx_to_symbol = dict(
            (atom.GetIdx(), oechem.OEGetAtomicSymbol(atom.GetAtomicNum()))
            for atom in mol.GetAtoms())
        print("Restraint atoms in OEMol:",
              [idx_to_symbol[idx] for idx in restraint_indices])

Expected behavior
The restraint indices should correspond to the correct atoms in the molecules from the dataset, and the output for the code above should be empty. Instead, I am getting the following output:
--------------------------------------
GridOptRecord id : 17247212
SMILES           : [H]C(=O)C([H])([H])N([H])C([H])([H])C(=O)[H]
Restraint indices: [ 4 13  2  3]
Number of atoms  : 11
Restraint atoms in OEMol: ['H', 'H', 'O', 'C']
--------------------------------------
GridOptRecord id : 17247224
SMILES           : [H]c1c(nc(c(n1)[H])N(C([H])([H])[H])C([H])([H])[H])[H]
Restraint indices: [8 3 4 5]
Number of atoms  : 7
Restraint atoms in OEMol: ['N', 'N', 'C', 'C']
--------------------------------------
GridOptRecord id : 17247254
SMILES           : [H]c1c(c(c(c(c1[H])[H])N([H])C([H])([H])[H])[H])[H]
Restraint indices: [ 7 16  5  6]
Number of atoms  : 9
Restraint atoms in OEMol: ['H', 'H', 'C', 'C']
--------------------------------------
GridOptRecord id : 17247255
SMILES           : [H]c1c(nc(c(c1N([H])C([H])([H])[H])[H])[H])[H]
Restraint indices: [ 7 15  4  5]
Number of atoms  : 15
Restraint atoms in OEMol: ['N', 'H', 'C', 'C']
--------------------------------------
GridOptRecord id : 17247256
SMILES           : [H]c1c(nnc(c1N([H])C([H])([H])[H])[H])[H]
Restraint indices: [ 7 14  3  4]
Number of atoms  : 12
Restraint atoms in OEMol: ['N', 'H', 'N', 'N']
--------------------------------------
GridOptRecord id : 17247263
SMILES           : [H]c1c(c(c(c(c1[H])[H])N(S(=O)(=O)C([H])([H])[H])S(=O)(=O)C([H])([H])[H])[H])[H]
Restraint indices: [ 8  5 13 14]
Number of atoms  : 12
Restraint atoms in OEMol: ['H', 'C', 'C', 'H']
--------------------------------------
GridOptRecord id : 17247278
SMILES           : [H]n1c(nnn1)N([H])[H]
Restraint indices: [5 7 8 0]
Number of atoms  : 6
Restraint atoms in OEMol: ['N', 'H', 'H', 'H']
--------------------------------------
GridOptRecord id : 17247279
SMILES           : [H]n1c(nnn1)N(c2nnnn2[H])c3nnnn3[H]
Restraint indices: [15  0  1  2]
Number of atoms  : 5
Restraint atoms in OEMol: ['N', 'H', 'N', 'C']
--------------------------------------
GridOptRecord id : 17247280
SMILES           : [H]n1c(nnn1)N([H])c2nnnn2[H]
Restraint indices: [10 13  0  1]
Number of atoms  : 9
Restraint atoms in OEMol: ['N', 'H', 'H', 'N']
--------------------------------------
GridOptRecord id : 17247282
SMILES           : [H]c1c(c(nc(n1)[H])[H])N(c2c(nc(nc2[H])[H])[H])c3c(nc(nc3[H])[H])[H]
Restraint indices: [18  9 10 11]
Number of atoms  : 14
Restraint atoms in OEMol: ['H', 'N', 'C', 'C']
--------------------------------------
GridOptRecord id : 17247283
SMILES           : [H]c1c(c(nc(n1)[H])[H])N([H])c2c(nc(nc2[H])[H])[H]
Restraint indices: [12 19  6  7]
Number of atoms  : 12
Restraint atoms in OEMol: ['C', 'H', 'N', 'H']
--------------------------------------
GridOptRecord id : 17247286
SMILES           : [H]c1c(n(c(c1N([H])c2c(c(n(c2[H])[H])[H])[H])[H])[H])[H]
Restraint indices: [10 19  6  7]
Number of atoms  : 12
Restraint atoms in OEMol: ['C', 'H', 'N', 'H']

Additional context
Python 3.7.3, Ubuntu 18.04
Library versions

qcelemental==0.11.1
qcengine==0.11.0
qcfractal==0.11.0
qcportal==0.12.1

Thank you for your detailed report. This might be related to #435. @dgasmith is the expert on this, but he's traveling today.
I would suggest updating to the latest qcportal (0.13.0), and seeing if that resolves the issue.

Taking your first example it appears to be correct:
>>> proc = client.query_procedures(id=17247212)[0]
>>> proc.keywords.scans[0].indices
[4, 13, 2, 3]
>>> client.query_molecules(id=proc.initial_molecule)[0].geometry.shape
(14, 3)

Please note that query order isn't guaranteed which is where items are going wrong in your code I believe. I highly recommend using queries like ds.query("default") which handle the ordering gracefully and correctly.

Thanks for the quick response from both of you! I am wondering, what is ds.query("default") supposed to do? When I use it, all I get back is the string default. I have read the docstring for the method, and this seems to make sense, but I am not sure what I should be doing with this output.

We should probably change that, but ds.query('default') populates the underlying ds.df DataFrame so that get_entry 1) becomes very fast and 2) correctly maps the entries to a record.
We should change the return to return df[spec.name] here.
@btjanaka Could you make a PR if you think that would make it more straightforward?

@dgasmith will do, thanks for the help!

Closed by #565.

As a followup, everything worked out when I tried again with ds.query. Here is the updated code:
import numpy as np
import qcportal as ptl
from openeye import oechem, oedepict
from tqdm.notebook import tqdm

# Loading Data
client = ptl.FractalClient()
ds = client.get_collection('GridOptimizationDataset',
                           "OpenFF Trivalent Nitrogen Set 3")
ds.query("default")

# Looking at the restraint indices in each record, and seeing whether they are
# available in the molecule
for index in tqdm(ds.df.index):
    ds_entry = ds.get_entry(index)
    record = client.query_procedures(ds_entry.object_map['default'])[0]
    molecule = client.query_molecules(ds_entry.initial_molecule)[0]
    smiles = ds_entry.attributes["canonical_explicit_hydrogen_smiles"]
    restraint_indices = np.array(record.keywords.scans[0].__dict__['indices'])
    num_atoms = len(molecule.symbols)

    # If any restraint index is out of bounds
    if np.any(restraint_indices >= num_atoms):
        print("--------------------------------------")
        print("GridOptRecord id :", record.id)
        print("SMILES           :", smiles)
        print("Restraint indices:", restraint_indices)
        print("Number of atoms  :", num_atoms)

        mol = oechem.OEMol()
        oechem.OESmilesToMol(mol, smiles)
        idx_to_symbol = dict(
            (atom.GetIdx(), oechem.OEGetAtomicSymbol(atom.GetAtomicNum()))
            for atom in mol.GetAtoms())
        print("Restraint atoms in OEMol:",
              [idx_to_symbol[idx] for idx in restraint_indices])

Awesome, that looks much more in line with what I hope our API can deliver.
Although, I highly recommend something like ds.get_record(entry_name, 'default'). Currently you will query the records twice with your upper level ds.query call.


docs(code-of-conduct): add code of conduct

Summary
Type

Documentation

We could reuse the same as scw-lib https://github.com/scaleway/scaleway-lib/blob/master/CODE_OF_CONDUCT.md


Telemetry for when using Event Grid Blob Trigger vs Blob Trigger

Currently, it is difficult for customers to determine when they are using the Event Grid Blob Trigger vs the standard Blob Trigger. We should add some additional telemetry specifying to customers which trigger is executing.

@anatoliB would you be able to triage/assign this item?

@AnatoliB would you be able to triage/assign this item?

Yes, thank you


Property serialization

I mentioned this in my issue I submitted: I'm not sure this is how you would implement this, or if even it's useful to anyone else, but I wanted to be able to apply a decorator to model method to flag it as a serializable property. These are sent along with the serialized model fields to the client.
To avoid conflict with other uses of the python property decorator, I created a ddp_property decorator that adds an attribute the method that marks it to be serialized. That bit is checked at the end of the serialize method.
I didn't have any luck receiving related data or annotated data through my subscriptions, so this was one solution. If there is a better way, please let me know. Thanks!

Thanks for the contribution! I like the concept, but I'm undecided on the implementation. Can you explain why this is preferred over having a common serialization method?

Yeah, I'm not sure. I assumed there was a better way to accomplish this.
I found that related objects only contain the ID of a related object, which would require me to look them up separately. I became accustomed to Django providing access to the entire related model, so I was trying to accomplish something similar in a single database pull. Am I missing something with this? Can we already pull down the values in related objects, or do I need to get them separately?
If I remember the code right, the serializer only handles the model fields and F expressions. I did try to annotate the query with related model values using F expressions, but it failed to serialize it in api.py. As a workaround, I just decided I would like to be able to decorate a model method and then have the serializer pick up the value it returns and append it to the payload going down to the client. This also allowed me to create properties that are accessible at the client-side as well.
With Django, I often use model methods as properties so I can reference them in the template or view easily. For example, how Django uses get_full_name on User model. This new decorator allowed for me to do this as well, so I thought it may be useful for calculated fields or similar.

Just after I posted my reply, I think I see something in the serialize method in api.py that I simply missed before. It looks like if we use select_related to define specific fields from the related model, they'll get serialized with the rest. Is that right?
If that's the case, I knew I was just missing something simple. Ugh...

@tysonclugg - did I understand the use of select_related in this case? And if so, could the property I created still be useful for calculated or dynamic "fields" that aren't a part of the core model or related model?


Add ROTATE stack operation

Part of #44
The ROTATE operation will move the bottommost element of a Stack to the top, becoming this the new peak. If the Stack is empty, it won't do anything.

Codecov Report

Merging #73 into dev-0.2 will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           dev-0.2    #73   +/-   ##
======================================
  Coverage      100%   100%
======================================
  Files           18     18
  Lines          829    862   +33
======================================
+ Hits           829    862   +33

Impacted Files
Coverage Δ

pilad/router.go
100% <ø> (ø)
:arrow_up:

pila/stack.go
100% <100%> (ø)
:arrow_up:

pkg/stack/stack.go
100% <100%> (ø)
:arrow_up:

pilad/conn.go
100% <100%> (ø)
: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 d64a5c9...7132c17. Read the comment docs.


Resuming suspended training

Hey folks,
Congrats on the amazing and inspiring work!!
I have a quick question - how do I resume training + wandb logging if the training got terminated before completion. E.g. say the original command was CUDA_VISIBLE_DEVICES=0 python -m train experiment=s4-lra-pathx loader.batch_size=16 trainer.accumulate_grad_batches=2 and the training got suspended halfway through the full training. What command should I run to resume the training and the wandb logging?
Thanks in advance,
Ankit

Hi thanks for the question!
You can resume training by passing in a path to the checkpoint using the trainer.resume_from_checkpoint flag. So for example,
CUDA_VISIBLE_DEVICES=0 python -m train experiment=s4-lra-pathx loader.batch_size=16 trainer.accumulate_grad_batches=2 trainer.resume_from_checkpoint=/path/to/checkpoint

This will start another wandb run that picks up where the last one left off. You'll want to point to the last.ckpt file that gets saved to the logging folder (your training logs should point you to where that is).

Thank you so much for the prompt response Karan! I'll follow your
instructions and let you know if I have any further questions!
Regards,
Ankit
On Thu, Feb 24, 2022 at 4:04 PM Karan Goel @.***> wrote:

Hi thanks for the question!
You can resume training by passing in a path to the checkpoint using the
trainer.resume_from_checkpoint flag. So for example,
CUDA_VISIBLE_DEVICES=0 python -m train experiment=s4-lra-pathx loader.batch_size=16 trainer.accumulate_grad_batches=2 trainer.resume_from_checkpoint=/path/to/checkpoint
This will start another wandb run that picks up where the last one left
off. You'll want to point to the last.ckpt file that gets saved to the
logging folder (your training logs should point you to where that is).
—
Reply to this email directly, view it on GitHub
https://github.com/HazyResearch/state-spaces/issues/7#issuecomment-1050263059,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AID3IGG2IAOZPOV4DI7SYNDU42MPRANCNFSM5PH7YPWQ
.
 That means that it wont copy the logs of the previous run and will appear as a partial run on the dashboard, right?

That's right: you should see a new (partial) run on the dashboard.


ENABLE_HARD_LINE_BREAK is not configurable

As you can see on the source the value of ENABLE_HARD_LINE_BREAK is currently not configurable as the mapping to the correct config key is missing, just the variable itself is referenced here: https://github.com/gogits/gogs/blob/master/modules/setting/setting.go#L116

See https://github.com/gogits/gogs/blob/master/modules/setting/setting.go#L415 ...

Ah, the full section is mapped at once. Sorry for the noise.

:trollface:


How to implement a gateway

Brief feature description
It would be really helpful, if there is an example gateway like there is an example avaialable for publisher and subscriber to see how a CaPro gateway needs to be implemented.
Detailed information
This could be a simple gateway to connect via TCP-IP or to connect two individual RouDis.

the documentation is quite sparse, especially on this topic. we are on it, though

The gateway functionalities are not yet described here. We are currently working on a gateway to cyclone dds. Once this is ready we should describe it like our examples


Add support for most commons HTTP errors

Hi everyone! I have been recently using Slim to build a ReST API.
Slim already provides handlers for 404, 405 and 500 errors, but to my understanding handlers set in this way are used internally by Slim.
If I wanted to return an error, currently, I'd have to interact with the response
return $response->withStatus(400)->withJson(/*some message*/);

What could happen instead is:
return Slim\Http\Errors::badRequest('my message')

This is only an example and  is just some syntactic sugar. For some errors it may also actually be useful, such as in case of 401 Unathorized where an header is also required.

Please explain more?

Sorry about that, accidentally pressed enter :S

So you can use custom exception handler and throw some kind of exception while your custom handler catches it and does what you want to do. :)

Maybe for 4.x


fix wps.describeprocess

This PR fixes wps.describeprocess. It filters the requested process from the available process list (before it returned just the first one). This is needed when an xml response document is given with several process descriptions in test cases.

@huard Maybe you like to have a look at it?

@huard thanks :)

Nice that you got the tests fixed. This was annoying.


"type must be boolean, but is object" error thrown on non-boolean object

parsing the included json string throws a json::type_error with the description  "type must be boolean, but is object"
OS: Linux Mint 20.3
Compiler: g++ 9.3.0
CPP standard: C++17
JSON string:
{
  "DistanceSensor": {
    "Distance": 217,
    "SensorType": "PING"
  }
}

minimal code example:
try
{
    json j;
    j.parse(jsonStr);
}catch(json::type_error& e){
    std::cout << e.what() << std::endl;
}

The code example you provided cannot throw that exception you mentioned - this occurs if you try to assign a JSON value of type text to a C++ bool - an example would be code like
json j = "text";
bool b = j;

Your example further call parse wrongly: it is a static function. You want to call
json j = json::parse(jsonStr);

oh wow I've been using this library super wrong, I fixed my blunders only to realize the type exception was from me forgetting that you can't null-check a non pointer object, thus leading to me using if(j) to determine if it parsed correctly before continuing, sleep before you program kiddos
good way to check if your json parsed properly for anyone that makes the same mistake as me:
           json j;

            bool parsed = true;
            try{
                j = json::parse(text);
            }
            catch(json::parse_error e)
            {
                std::cout << e.what() << std::endl;
                parsed = false;
            }

            if(parsed)
            {
            }


ui-sref-active doesn't respect deferIntercept

Following on from this issue: https://github.com/angular-ui/ui-router/issues/1807
I've documented this solution for first-page server side renders:
https://stackoverflow.com/questions/29446178/prevent-angular-ui-router-from-re-loading-ui-view-on-first-page-load/29453450#29453450
The only issue seems to be that ui-sref-active is not also deferred. The server renders the tab with a class="active" on the first page load, but ui-sref-active removes the class when it is loaded.
Is the right solution to this problem modifying the directive to respect the deferred?

Sounds like that might be the case. Should I refer to the plunk linked in #1807?

I've made an updated plunkr! http://plnkr.co/edit/HZHJlGEqKeKgPJcPChho?p=preview

@aidanlister Sounds good, thanks for that. I'll bang on this as soon as I have some time. Just to make sure, the console output is:
defer
main controller activated

...and it should only be showing defer, correct?

Hrm, I don't know how the controller should work. My main concern is that ui-sref-active is stripping the active class when it shouldn't be. This is a common case for serving the first page load via server side rendering, and letting angular take over after that. In that scenario it makes sense to let the controller run, but to defer sref-active from running in the same way that the ui-view is deferred.

Closing due to inactivity


Strange error in console.

While loading rocket commands:

Its not an error nor has any impact, can be ignored.

ok, got it, thanks :+1:


Updated the spanish translation

fixing some typos, and doing some corrections

@victorhck I suggested a couple of changes based on the english meaning, if you can please take a look at them and if you agree on my proposals we can merge it, or maybe you have better understanding for the words you propose.
Thank you.

Argg, I have just seen this after creating a merge request. D'oh!


Brgin ModerFlyout To powertoys

This app is still in preview but its so cool i think it could be a part of power toys.
https://github.com/ModernFlyouts-Community/ModernFlyouts
And store version:
https://www.microsoft.com/en-us/p/modernflyouts-preview/9mt60qv066rp?ocid=badge&rtc=1

Duplicate of https://github.com/microsoft/PowerToys/issues/6273


Default mapping from Symbol?

Hi @gdalle,
Awesome package, thanks for making it!
I am considering whether to use this as the backend interface of SymbolicRegression.jl to allow the user to pick an AD backend (current attempt). However, one thing I was wondering if you would consider is designating a mapping from Symbol to the correct AbstractADType? I could implement this within my package, but it seems like something that should live here for others as well.
Basically, I would prefer that my users do not need to install both Zygote.jl and DifferentiationInterface.jl (or ADTypes) in their environment to set Zygote as the backend. I would rather they just pass, e.g., :Zygote and then have my package, which depends on DifferentiationInterface.jl, be able to internally map to the right AbstractADType.
Within DifferentiationInterface.jl this could look like:
function lookup_backend(key::Symbol)
    return ADTYPE_MAP[key]
end

const ADTYPE_MAP = Dict{Symbol,AbstractADType}([
    :Zygote => AutoZygote(),
    :Enzyme => AutoEnzyme(),
    :ForwardDiff => AutoForwardDiff(),
])

Then within my package, I can have my Options constructor look like this:
using DifferentiationInterface: lookup_backend, AbstractADType

function Options(autodiff_backend::Union{AbstractADType,Symbol})
    backend = autodiff_backend isa Symbol ? lookup_backend(autodiff_backend) : autodiff_backend

    return Options{typeof(backend)}(backend)
end

which means the user has a more lightweight interface; they can just pass the symbol:
using SymbolicRegression, Zygote

options = Options(:Zygote)

And then I map to the right type internally.
I guess I could also re-export all the abstract types but it feels a bit overkill. That would also mean I would be unable to provide helpful error messages if they forget the name of the backend and pass in the wrong symbol.
What do you think?
Cheers,
Miles

However, one thing I was wondering if you would consider is designating a mapping from Symbol to the correct AbstractADType?

 That's why e.g. Optimization.jl switched to ADTypes.jl. The reasons are described in more detail in the ADTypes.jl README.
I fear that adding a layer of translation from Symbols to ADTypes.AbstractADType would only result in competing standards, as well as suboptimal default choices for all the things a Symbol cannot express.

Basically, I would prefer that my users do not need to install both Zygote.jl and DifferentiationInterface.jl (or ADTypes) in their environment to set Zygote as the backend.

Why is that? If they use Zygote.jl as the backend, it will be installed anyway. And AD backends are infinitely heavier than ADTypes.jl or DifferentiationInterface.jl. ADTypes.jl has zero dependency on Julia >= 1.9, and DifferentiationInterface.jl is very lightweight too:
https://github.com/gdalle/DifferentiationInterface.jl/blob/fa551e53ffeff7f20ca126bfda380d2731b0e100/DifferentiationInterface/Project.toml#L6-L14
To me, the following interface is even better, because it does not introduce a separate Symbol mapping, and instead leverages ADTypes.jl directly. Keep in mind that users may already be familiar with this system if they have ever used a SciML package.
using ADTypes, SymbolicRegression, Zygote

options = Options(AutoZygote8))

And by the way, I'd be thrilled to help you get DifferentiationInterface.jl up and running with SymbolicRegression.jl. Don't hesitate to ping me for reviews or advice in that PR

Thanks!
It's not so much the extra dependency as SymbolicRegression.jl would of course already depend on DifferentiationInterface.jl if this goes through. The issue is I would be asking beginner users to install a third package in their environment, even after I have already asked them to install the AD backend package itself, just so they can set one of the parameters of the search.

I agree with this, in the example I gave above I allow both AbstractADType and a Symbol.
using DifferentiationInterface: lookup_backend, AbstractADType

function Options(autodiff_backend::Union{AbstractADType,Symbol})
    backend = autodiff_backend isa Symbol ? lookup_backend(autodiff_backend) : autodiff_backend

    return Options{typeof(backend)}(backend)
end

So advanced users can install ADTypes and customize the backend as they desire, but beginner users (which SymbolicRegression.jl gets a lot of from the Python frontend PySR) still can just pass in a :Zygote – which is probably good enough in 95% of cases.
(Even package extensions themselves I find to be a distraction for beginners, which is why PySR just automatically installs stuff as the user needs it – https://github.com/MilesCranmer/PySR/blob/89e991d9d7a5e34025bdbbe9076a8fd0892f04cc/pysr/julia_extensions.py#L8-L36 – rather than asking the user to install each package themselves. I just find it to be another barrier for people using my package.)

I understand your point, and I do think it makes a lot of sense to offer an easier setting, especially for people coming from Python. But I don't think this Symbol mapping should be part of DifferentiationInterface.jl, if only because I try to make as few assumptions as possible.
The best of both worlds might be to automatically turn a Symbol into an ADTypes.AbstractADType as follows: :Package -> ADTypes.AutoPackage(). In most cases this will work (except for a few minor backends which always require arguments) and yield a reasonable backend object.
That way you don't have to maintain an explicit mapping, and you automatically benefit from e.g. new backends that might be added to ADTypes.jl.
julia> using ADTypes

julia> s = :Zygote
:Zygote

julia> auto_s = Symbol(:Auto, s)
:AutoZygote

julia> backend = @eval $auto_s()
AutoZygote()

The reason I thought it might make sense to offer such a mechanism in this package is so that we can avoid having different ways of solving this. I am guessing that other packages may want to offer similar functionality in the future. So it would be nice to have a unified mapping from symbol to backend type.
Even something like
function Auto(key::Symbol; options...)
    backend_type = lookup_type(key)
    return backend_type(; options...)
end

Which would let me pass extra configuration steps as well.
Then I can import this function internally in my package, and allow the user to pass a symbol and keyword backends to my options constructor, without needing to manually install the ADTypes.

I guess I feel like (symbol; kws...) -> Auto{symbol}(; kws...) is quite an innocent choice?
You could even make it like
function Auto{key}(; options...)
    backend_type = lookup_type(key)
    return backend_type(; options...)
end

rather than needing AutoEnzyme, AutoZygote, etc., which need to be explicitly imported.
So, to summarise my thoughts:
The issue with having the user install ADTypes.jl themselves is that it presents a small barrier to usage and more mental overhead. While it wouldn't slow down performance, it would mean another line in the user's Project.toml file, the extra effort of needing to run add ADTypes, as well as simply the user needing to figure this out in the first place, rather than simply passing in :Zygote which I can document pretty easily.
The two workarounds are:

Re-export ADTypes myself.

The issue with this is that I would either need to re-export all types (which is dangerous, in case of any future conflicts), or re-export the current set of types (meaning I would need to keep this up-to-date manually).

Write my own mapping from symbol => type.

This has the same issue of needing to keep this up-to-date manually with all of the types in ADTypes
This also introduces the problem of proliferating multiple standards for doing this, leading to a similar problem to the one this package was designed to solve.
If I get around this by using the ADType.eval(symbol("Auto", ad_backend)) trick, it is a bit dangerous (eval in general runs into https://en.wikipedia.org/wiki/Arbitrary_code_execution), and also might lead to issues with precompilation due to manual calls of eval

The reason I thought it might make sense to offer such a mechanism in this package is so that we can avoid having different ways of solving this. I am guessing that other packages may want to offer similar functionality in the future.

From my POV, this is why ADTypes exists in the first place–it is the standard you are asking for. ADTypes are basically symbols with optional configuration options:

A natural approach is to use a keyword argument with e.g. Bool or Symbol values.
Let's see a few examples to understand why this is not enough:

autodiff = true: ambiguous, we don't know which AD package should be used
autodiff = :forward: ambiguous, there are several AD packages implementing both forward and reverse mode (and there are other modes beyond that)
autodiff = :Enzyme: ambiguous, some AD packages can work both in forward and reverse mode
autodiff = (:Enzyme, :forward): not too bad, but many AD packages require additional configuration (number of chunks, tape compilation, etc.)

A more involved struct is thus required, with package-specific parameters.
If every AD user develops their own version of said struct, it will ruin interoperability.
This is why ADTypes.jl provides a single set of shared types for this task, as an extremely lightweight dependency.
They are types and not enums because we need AD choice information statically to use it for dispatch.

As Guillaume mentioned, there are some very large packages putting their weight behind ADTypes as a unified interface to select backends:

Keep in mind that users may already be familiar with this system if they have ever used a SciML package.

 Regarding this:

Maybe reexporting ADTypes is an option if you don't want users to have to load the package explicitly?

It's a bit tricky, because either I re-export everything in the package (which is a bit dangerous for a few reasons), or I keep a list of current AD types myself, and regularly update it.

My issue is that if I make this translation available in DI (or in ADTypes itself for that matter), I somehow endorse it and encourage people to use it. But I don't want to, because there should only be one standard. Additionally, starting from a Symbol makes everything else type-unstable, unlike a properly defined AD backend.

Additionally, if you ever want to use our sparse AD functionality[^1], the amount of symbols will grow combinatorially.
[^1]: Would be interesting to discuss whether this could have some applications in symbolic regression. We're still on the look-out for possible applications!

Additionally, starting from a Symbol makes everything else type-unstable, unlike a properly defined AD backend.

Actually this wouldn't be an issue.
First, this wouldn't redefine anything, it's just making an mapping that packages can use for easy user-side APIs. But a user would always be able to pass in a full ADType instead to the options if they wish.
Second, if the symbol is fixed in some code, Julia would just inline it and it would be stable.

Additionally, if you ever want to use our sparse AD functionality1, the amount of symbols will grow combinatorially.

Actually this wouldn't be an issue – the symbol here would only be the type specification. Like this:
function Auto(symbol, options...; kws...)

    if symbol == :Zygote
        AutoZygote(options...; kws...)
    elseif symbol == :Enzyme
        AutoEnzyme(options...; kws...)
    end
end

So, in your example, it would be
Auto(
    :Sparse,
    Auto(:ForwardDiff);
    sparsity_detector=TracerSparsityDetector(),
    coloring_algorithm=GreedyColoringAlgorithm(),
)

which could be done without needing to re-export all the ADTypes.

By the way, just curious, why not have
const AutoZygote = Auto{:Zygote}
const AutoSparse = Auto{:Sparse}
const AutoEnzyme = Auto{:Enzyme}

instead of AutoZygote, AutoSparse, etc? This would also solve the problem as I can simply map the user's passed symbol to Auto{symbol} inside my constructor, rather than need the user to install and import AutoZygote themselves.
I mean in general I should say I don't feel too strongly about this. It just seems like it would be useful enough for designing APIs.
I note that will have to do some symbol => type mapping anyways, because of PySR (users can't specify Julia types there).

This also wouldn't be an issue – the symbol here would only be the type specification. Like this:
function Auto(symbol, options...; kws...)

   if symbol == :Zygote
       AutoZygote(options...; kws...)
   elseif symbol == :Enzyme
       AutoEnzyme(options...; kws...)
   end
end

So, in your example, it would be
Auto(
   :Sparse,
   Auto(:ForwardDiff);
   sparsity_detector=TracerSparsityDetector(),
   coloring_algorithm=GreedyColoringAlgorithm(),
)

You do realize that this is not exactly diminishing the mental load of the user ;) This is essentially reinventing ADTypes with symbols.

By the way, just curious, why not have
const AutoZygote = Auto{:Zygote}
const AutoSparse = Auto{:Sparse}
const AutoEnzyme = Auto{:Enzyme}

instead of AutoZygote, AutoSparse, etc?

For the same reason that symbols are not specific enough, symbol wrappers won't work either. I encourage you to take a look at the ADTypes documentation if you want to understand what we actually put inside each backend struct, and why it matters.

I note that will have to do some symbol => type mapping anyways, because of PySR (users can't specify Julia types there).

To me this is the main (or only) argument in favor of symbols: calling Julia autodiff from Python or other languages. Everything else can be solved from within Julia with minimal efforts, but this seems to be a hard limit.

Although even that limit can be overcome with juliacall, right?
from juliacall import Main as jl
jl.seval("using ADTypes")
backend = jl.AutoZygote()

You do realize that this is not exactly diminishing the mental load of the user ;) This is essentially reinventing ADTypes with symbols.

Sorry I was just giving that as an example in regards to the comment that "amount of symbols will grow combinatorially". For that example in particular you would probably just want the user to install ADTypes, so that they can get really custom with it. Which my request is not opposed to!
Advanced users are not as sensitive to mental overheads :) It's really just the newbies where it would be great to have shortcuts for.
In the cases where the user is just selecting a backend like enzyme or zygote or forward diff,  just allowing a symbol would be nice. And then for the reasons to solve this here rather than elsewhere, I reference my comments in https://github.com/gdalle/DifferentiationInterface.jl/issues/319#issuecomment-2172893426.

Although even that limit can be overcome with juliacall, right?

Indeed it can. But,

I want my beginner users to try out custom backends as well – for them, I don't want to ask them to write Julia code unless they are really doing custom stuff. Picking a backend can 95% of the time be a single symbol, so I think that would be nice.
As your example shows, asking new users to do custom stuff with juliacall can result in some unexpected issues: Importing juliacall before PySR skips the default configuration that PySR imposes: https://github.com/MilesCranmer/PySR/blob/master/pysr/julia_import.py. (This is technically an issue with juliacall itself, in that there's no way to provide env-specific configuration, but it also shows the reason I don't want to ask pure-Python users to mess around with juliacall :))

For the same reason that symbols are not specific enough, symbol wrappers won't work either. I encourage you to take a look at the ADTypes documentation if you want to understand what we actually put inside each backend struct, and why it matters.

Sorry I am going off topic here but couldn't you just do this as follows?
struct Auto{S,C} <: AbstractADType
    options::C
    Auto{S}(; options...) = (nt = NamedTuple(options); new{S,typeof(nt)}(nt))
end
Base.getproperty(a::Auto, k::Symbol) = getproperty(getfield(a, :options), k)

const AutoZygote = Auto{:Zygote}
const AutoEnzyme = Auto{:Enzyme}
...

Since options are stored in a named tuple, all the relevant info is typed, no? This should even be backwards compatible I think.

That's actually not a bad idea for ADTypes.jl in the long run. In the meantime I opened https://github.com/SciML/ADTypes.jl/pull/62 to see what other maintainers think. Can you play around with that branch in your DI integration attempt, and let me know how it works?

Sounds good to me

Apart from this issue, which is getting solved, any other troubles when trying to integrate DifferentiationInterface.jl? What are you using it for?

Basically just to let the user choose what AD backend is used during constant optimization. The only issues left are just from the different backends being incompatible (only Zygote/ChainRules works at the moment), but this is the fault of other backends, not the interface.
(I’m not sure if Enzyme will work though, as usually I would have to declare what variables are being used as mutable storage?)

Basically just to let the user choose what AD backend is used during constant optimization.

Note that for some backends you'll get an epic performance boost by using preparation, especially in small dimension.

The only issues left are just from the different backends being incompatible (only Zygote/ChainRules works at the moment)

Does ForwardDiff work at least?

I’m not sure if Enzyme will work though, as usually I would have to declare what variables are being used as mutable storage?

Well, the whole point of DifferentiationInterface is that you get to try it out for free!
Using Enzyme optimally from DI is very hard without making DI just a frontend for Enzyme. But if you have an example for which it fails, don't hesitate to open an issue here.
Related discussions:

https://github.com/gdalle/DifferentiationInterface.jl/issues/311

Note that for some backends you'll get an epic performance boost by using preparation, especially in small dimension. What is the typical number of constants you optimize?

I usually can't use preparation because the gradients change for every expression. But the number of constants is typically ~1-10. For parametrized expressions (the reason I'm adding Zygote support) it can be up to 100.

Does ForwardDiff work at least?

For ForwardDiff I unfortunately have to use a separate function because it requires changing the element type.

Well, the whole point of DifferentiationInterface is that you get to try it out for free!
Using Enzyme optimally from DI is very hard without making DI just a frontend for Enzyme.

Yeah, indeed it seems hard to do so. I get the following when I try –
nested task error: Enzyme execution failed.
            Mismatched activity for:   %value_phi10 = phi {} addrspace(10)* [ %17, %L49 ], [ %getfield, %L46 ] const val:   %getfield = load atomic {} addrspace(10)*, {} addrspace(10)* addrspace(11)* %16 unordered, align 8, !dbg !211, !tbaa !215, !alias.scope !189, !noalias !210, !dereferenceable_or_null !216, !align !217
            Type tree: {[-1]:Pointer}
             llvalue=  %getfield = load atomic {} addrspace(10)*, {} addrspace(10)* addrspace(11)* %16 unordered, align 8, !dbg !211, !tbaa !215, !alias.scope !189, !noalias !210, !dereferenceable_or_null !216, !align !217
            You may be using a constant variable as temporary storage for active memory (https://enzyme.mit.edu/julia/stable/faq/#Activity-of-temporary-storage). If not, please open an issue, and either rewrite this variable to not be conditionally active or use Enzyme.API.runtimeActivity!(true) as a workaround for now

            Stacktrace:
             [1] getproperty
               @ ./Base.jl:37
             [2] getindex
               @ ./subarray.jl:323
             [3] set_constants!
               @ ~/PermaDocuments/SymbolicRegressionMonorepo/DynamicExpressions.jl/src/NodeUtils.jl:93

I wonder if there's some generic way to declare storage in a way that would pass the required info to Enzyme as well as any future backend that might require it? Otherwise will need to have an if-statement over backend which seems non-optimal. Especially because performance is so key to my use-case.

For ForwardDiff I unfortunately have to use a separate function because it requires changing the element type.

I'm curious now, why can't your code be type-agnostic?

Yeah, indeed it seems hard to do so. I get the following when I try –

Do you have a full reproducible example?

I wonder if there's some generic way to declare storage in a way that would pass the required info to Enzyme as well as any future backend that might require it?

This is related to, but different from, the issue about supporting active and inactive variables. And it's exactly what I said about Enzyme earlier: it is so different from other backends, and offers so many more options, that if I want to include every one I'll turn DI into a shallow frontend for Enzyme only.

Cool!

Alright then, I'm gonna close this issue in favor of the more specific #551, don't hesitate to open new ones!


Grammar in README.md

Proposed fix: #147.

My apologies for using the wrong name for Nuxtent. Here's an updated fix: #149.