-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
0.6.7 breaks resumables with multiple fors over same variable #86
Comments
Thank you for the report and my apologies for the frustration this probably caused. 0.6.6 had some black magic related to inference that caused some bugs in edge cases so we had to remove it (for now). If you have the bandwidth, could you please consider reporting whether your code works on the following:
Also, if you have a public library that depends on ResumableFunctions, let us know, we will add it to our CI to avoid unintentionally breaking it in the future. |
Yeah, took a bit of effort and head scratching to figure out. ;) Quick testing of the above examples shows that it indeed only works with 0.6.6 (with both 1.9 and 1.10). I can try with the original code (unfortunately not public) next week, but don't expect different results. |
In 0.6.6 we had a very talented contributor submit a much better inference/compiling pass for the resumable function macro. However, it had some minor regressions in performance that were not discovered until after julia 1.10 became public. We had to undo his contribution because of these regressions and he has not had a chance to work on it again since then (early January). It seems the functionality you needed was only available in that improvement from 0.6.6. We would like to get that improvement back in, but we are a bit starved for time right now, so probably it would not happen at least for a few more weeks. |
Sure, there is no rush. I've pinned it to 0.6.6 for now. B.t.w., via #81 I saw that QuantumSavory/QuantumSavory.jl#89 seems to describe the same issue. So perhaps that package is a candidate for inclusion in your CI. And here is a simpler example of the issue that can perhaps be included in your tests.
As far as I can tell, the issue is that ResumableFunctions.jl/src/utils.jl Line 61 in fdfbc51
and ResumableFunctions.jl/src/utils.jl Line 84 in fdfbc51
This works for me and does not reintroduce the other issue. Do note that this still leaves the issue that Related to that, the documentation is a bit confusing on this topic as it kinda suggests that the return value of the function is also yielded. Might be a good idea the clarify that calling the "iterator" directly gives both the yielded and return values, while using it as a proper iterator, it only returns the yielded values. |
…e functions) and #86 (repeated variable names)
…e functions) and #86 (repeated variable names)
This will be shortly fixed in #88 |
After #88 is merged (or even right now), please do not hesitate to submit issues about unclear documentation or better type inference. |
…e functions) and #86 (repeated variable names)
From version 0.6.7, the code below fails with
ERROR: LoadError: MethodError: Cannot
convertan object of type String to an object of type Nothing
. It works fine with 0.6.6.This happens because the same variable is used for multiple loops, and the type of that variable/slot in the generated iterator is determined by the last loop and only through code analysis, not the signature of
_empty
. This means thats
is types asNothing
, causing the mentioned error in the first loop on actual assignment tos
.The same happens with the (undocumented)
@yieldfrom
:The text was updated successfully, but these errors were encountered: