-
Notifications
You must be signed in to change notification settings - Fork 156
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
Issue #961, solve_for outputting wrong answers when function is nonlinear. #1101
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1101 +/- ##
==========================================
- Coverage 77.07% 76.60% -0.47%
==========================================
Files 32 35 +3
Lines 3533 3693 +160
==========================================
+ Hits 2723 2829 +106
- Misses 810 864 +54 ☔ View full report in Codecov by Sentry. |
Tests fail |
I'll take a look at it asap. |
… in case they happen, so now the function relies on linear_expansion for these cases
for eqᵢ in eq | ||
try | ||
islinear &= Symbolics.isaffine(eqᵢ.lhs-eqᵢ.rhs, var) | ||
catch e | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of try/catch, the root issue should be handled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed. this is just a temporary fix. Want me to open an issue on the failed tests for isaffine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, the current PR temporarily solves issue #961 and passes all the CI tests (i think the integration tests will fail nontheless, correct me if i'm wrong)
The
linear_expansion()
function seesx*y ~ 0
as linear due to looping over the lhs variable by variable, while observing the other variables as a constant. For example, when observing the x variable it sees its coefficient as a constant y, not as another variable. As such, it outputs a wrong bool (true). This hinders with the accuracy of the solve_for function as seen in issue #961.In order to mitigate this, isaffine can be used since it provides more accurate results, and an error message is thrown when the equation given is nonlinear since solve_for does not currently have the capabilities to solve the equations given in issue #961.