-
Notifications
You must be signed in to change notification settings - Fork 117
Create TemplateExpression for providing a pre-defined functional structure and constraints
#355
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
Conversation
|
Very odd. It's extremely worrying if there's an aliasing issue though. I can't immediately see where it might be coming from. |
|
It looks like sometimes, crossover attempts to cross the same expression with itself. Perhaps that is why this issue arises? Not because of the fact that there is aliasing, but because Yep, it looks like crossover applied to the exact same population member is enough to cause aliasing! Because it will put both "children" back into the population. If that's the same member, then that will mean that multiple population members are the same! |
BlueprintExpression for providing a pre-defined functional structure and constraints
Benchmark Results
Benchmark PlotsA plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR. |
Pull Request Test Coverage Report for Build 11394658450Details
💛 - Coveralls |
|
|
BlueprintExpression for providing a pre-defined functional structure and constraintsTemplateExpression for providing a pre-defined functional structure and constraints
TemplateExpressionThis lets you easily create expressions with a specific structure.
For example, if you want to have$$f(x_1, x_2) + exp(- g(x_3) \cdot g(x_3))$$ , this PR will let you.
There are some remaining issues though that we need to fix. It seems there is an undefined reference error when simplifying that I think is likely due toget_treestitching the inner expressions together, whereby some function (that I haven't figured out yet) mutates the resultant expressions.This should never happen, and is always avoided bymutate!because of the fact that they call the new functionget_contents_for_mutationwhich prevents this. But I guess there are some other functions which also need this guard in place.Fixes aliasing issues
This was a very bad bug that I just happened to identify as part of this PR ([1],[2]). Basically, the crossover operation would sometimes happen between a population member and itself. There was no check to prevent the crossover from sampling the exact same member from the population!
In addition to weird effects from actually performing a crossover operation on the same expression, this also meant that multiple individuals in a population could end up being the same. This means that all downstream operations (including ones ran in parallel!) would be susceptible to aliasing issues.
In short, this was a nasty bug, and it is surprising that it only showed up explicitly when implementing the BlueprintExpression. I wonder if this was the source of various crashes (especially for long-running jobs) over the past 1+ years, and I was just never able to identify the source of the race condition.
Anyways, this seems like it is fixed now, because we copy population members immediately after sampling.
Fix
maxsizeissuesThere were some issues hanging around that had to do with
maxsizebeing violated. It turns out this was simply an issue of a<appearing where a<=should have been. This has been fixed.TODO:
ReadOnlyNode, which throws an error for anysetproperty!. Callingget_treeonConstrainedExpressionwill return this node type.ReadOnlyNodeforStructuredExpressionget_treeaccess SymbolicML/DynamicExpressions.jl#105ConstrainedExpressionbut I'm not a fan of this.ConstrainedStructuredExpressionis too long. MaybeMultiExpression?