[Nonlinear] add support for simplifying NonlinearFunction#2605
[Nonlinear] add support for simplifying NonlinearFunction#2605
Conversation
|
Nice, this seems very useful |
|
If we decide to add more algebraic simplifications in future, they won't be considered breaking right? Definitely looking forward to this! |
|
Given the various bits I'm having to poke and prod, I'm not sure if this is a good or bad idea to have on by default. |
| # Use of this source code is governed by an MIT-style license that can be found | ||
| # in the LICENSE.md file or at https://opensource.org/licenses/MIT. | ||
|
|
||
| module SymbolicAD |
There was a problem hiding this comment.
Why is this called SymbolicAD ? It's not really doing AD, it's just simplifying
There was a problem hiding this comment.
My plan was to move lanl-ansi/MathOptSymbolicAD.jl#39 into here
| function simplify(f::MOI.ScalarQuadraticFunction{T}) where {T} | ||
| f = MOI.Utilities.canonical(f) | ||
| if isempty(f.quadratic_terms) | ||
| return simplify(MOI.ScalarAffineFunction(f.affine_terms, f.constant)) |
There was a problem hiding this comment.
Calling simplify will canonicalize again, we don't need to do the type unstable part of deciding whether it should be a constant.
There was a problem hiding this comment.
I don't understand this comment
There was a problem hiding this comment.
we just did f = MOI.Utilities.canonical(f) so f.affine_terms are already canonicalized. Here, we create a SAF that we know is canonical and then pass it to simplify. The first thing the function will do is canonicalize again which is a bit wasteful
|
I went back to making some changes in lanl-ansi/MathOptSymbolicAD.jl#39 for now |
|
Closing. I'll open something else. |
3 participants
Heading towards #2553
We have a few choices:
Utilities.canonicalizeisapproxNot really sure what is a good approach. To be honest,
canonicalize!(::ScalarNonlinearFunction)already does more than I remember adding.