Full Rearrangement Support#9
Open
RLee64 wants to merge 16 commits into
Open
Conversation
- Removed curly braces where unnecessary - Changed explicit type declaration to auto - Renamed symbolExpr to symbol
This was painful even with copilot's help........
Previously our conversion to simplify -1 * x to -x didn't account for multiplication expressions with more than 2 children (e.g. -1 * x * y), this led to incorrect conversion (i.e. -x - y). This commit fixes this issue, and also structures the function a bit more clearly in general.
As seen in trigonometric functions, symengine is not capable of inversing transcendental functions. So no such test cases for those have been created.
In order to achieve this, we also have to adjust how our map is populated. Since we don't have access to the voi from an individual equation, we'll instead populate variables in our rearrangement map as we encounter them.
It's very difficult for us to try and make sure every equation we parse into a seEquation is rearrangeable. As such we'll let SymEngine throw errors and catch them to make sure a failed rearrangement doesn't nuke the analyser's ability to do its job.
We want to make sure they fail at rearranging and produce an NLA system
Experienced an issue where the symbol map doesn't account for when the rearrangement subject can have multiple names (which most likely occurs when the variable the equation is solving for is located in a different file). This was discovered due to generator.sineImports failing (test case still fails, but for another reason now)
Hopefully this change also fixes the issue for Mac/Linux (0_0 )
This is an issue that only occurs in symengine debug mode since symengine wants to make sure our derivatives match their canonicality requirements. The problem seems to be that placing symengine requires the independent variable to be in a function with the dependent variable before differentiation. Since this makes things difficult to convert to and from, I've instead opted to make differentiation a function symbol since we're not trying to rearrange for the differential anyway.
Again, symengine flags a canonical issue, but this time it's because when attempting to solve for an error, the internal symengine tries to create a non-canonical POW element. This completely forces our hand since symengine aborts the program after a canonical error, which means we can't catch the issue like in other cases. As a temporary measure I've removed the test case, but this is by no means ideal.
Need to review this in greater detail along with the others, but it seems the extended rearrangement capabilities have modified some of our equations to eliminate certain NLA systems.
Reverts a line from e1d91b1
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Overview
Pull request addressing #5, this aims at extending functionality to support all reasonable rearrangement problems that SymEngine is capable of handling.
Changes
symbolMapandvariableMapwas adjusted to be created dynamically as we found variables. This allows us to keep track of the variable of integration as well.Notes
Failing Test Cases
Generator.sineImportsas well. These models seem to all have some of their equations rearrangements, consequently altering their generated outputs, which ultimately fails the test checking output equivalence.