[DO NOT MERGE] Parameter Namespaces [Variant A] #400
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…he allow_partial_parameter_mapping argument. parameters which are not mapped by the parameter_mapping dict passed to MappingPT are automatically mapped using identity mapping.
Added mapping_namespace argument to MappingPT. All parameters of the inner template not explicitely mapped by the parameter_mapping provided to MappingPT are placed into the provided namespace by mapping them to parameters named <mapping_namespace>___<original name>. If the parameter already was in a namespace (i.e. had a prefix ending with ___) this is replaced by the new namespace/prefix.
…pressions. Created customized version of the auto_symbol token transformation used by sympy.parse_expr and mocked that into sympy when calling sympify and co. Works well for sympify but not at all for lambdified and compiled expressions. An issue for lamdified expression is that the namespaced parameters will become kwargs and python cannot deal with the dot in there. anything to be done? WIP? probably just scrap this..
Expression evaluation with sympy now works without issues, however, parameters passed into evaluation now have to have "___" as namespace separator. This could/should be automated away somewhere (parameter provider class?).
…notation. Now, users only see namespaced parameters as "foo.bar" while sympy only sees them as "foo___bar".
- fixed transformation routine itself
- caught some calls to sympy that were not mocked
- switching between dot and internal underscore notation where required
- added additional underscore to internal namespace seperator ('____') to prevent existing pulses using the 3-underscore notation to be wrongly mapped to dot notation
- expanded utils.sympy_tests to also work on namespaced symbols (still errors left as of now, not complete 'coverage' yet) - removed tests that tested native sympy functionality (why were we testing this?)
cleaned up tests file
|
I also copied the patched auto_symbol method into a sympy build and ran all sympy tests to guarantee that it doesn't break existing behavior. Currently, there are a few errors which need to be addressed. |
…method of PulseTemplates. - added function substitute() in sympy and subs() in ExpressionScalar which symply wrap calls to sympy.Expr.subs but rename namespaced parameters - adapted integral() methods in PulseTemplates to accomodoate for those changes - added tests for namespaced parameters in integral() methods for PulseTemplates
Pull Request Test Coverage Report for Build 1615
💛 - Coveralls |
One of the errors comes from a test which evaluated showing clearly that the dot '.' can be (and IS) used in sympy expressions to access object attributes. This basically makes using the dot as a separator for parameter namespaces impossible for us. However, expressions in qupulse can probably live without this behavior just as well as their are primarily intended to convey mathematical meaning and not python(-ish) code. |
…master. - had replaced expression.subs() with recursive_substitution() since the first one did not rename namespaced parameters, which is not replaced with substitute() which is the namespace-aware wrapper of expression.subs() - sympy.py: replaced a failure-catching if statement which should never occur with an assert() - added kwargs to utils.sympy.substitute() and ExpressionScalar.subs()
…ite. (WIP) our tests break for subscriptable symbols now
custom_auto_symbol_transformation now again ignores local/global symbols that are followed by a namespace separator (.) and interprets it as symbol
lumip
changed the title
lumip
changed the title
|
A discussion with @pcerf @reneotten and others resulted in choosing Variant B ( #418 ). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Functionality to put parameters in namespaces (in a syntactically nice way);
monkey patched sympy's auto_symbol token transform parsing step to allow symbol names with dots (e.g. "foo.bar", "foo.bar.bar") to signalize namespaces for parameters (instead of python object attribute access).
internally these will be replaced with underscore namespace notation
foo____bar____bar(python prevents us from keeping the dots: symbol names are regularly pythoneval'd or used as keyword arguments in lamdification -> Syntax errors)evaluation and substitution code in utils.sympy transparently renames incoming parameter dictionaries from dot to underscore notation (and back) if required. users only need to face dot notation.
compared to Variant B, this has the following advantages:
foo.bar.blainstead ofNS(foo).NS(bar).bladisadvantages:
we currently favor Variant B, any active development will be happening there