Repository navigation
Conversation
|
Here is Opus 5.5's opinion on this: Right now there are three different rules on master:
The bridges look like they only cover the mutation path that #2328's test checks. The objective container covers only the other one. Proposal: a model owns what it stores, and it only has to copy what it could mutate.
Why copy in both directions isn't silly:
Changes needed to apply it:
The alternative is to make add_constraint take ownership, so the caller must not touch f afterwards and nothing is copied on the way in. That saves one linear copy per constraint, but it turns the two-constraints-sharing-f case into silent corruption, so I'd keep copying on the way in. |
|
So that confirms the motivation for this PR, we should copy both in |
|
Another inconsistency: |
* StructOfConstraints did not forward UnsafeConstraintFunction so Utilities.Model returned a copy. * The ObjectiveContainer getter of ObjectiveFunction calling UnsafeObjectiveFunction was never called as the existing getter of ObjectiveFunction was more specific. The existing getter now implements UnsafeObjectiveFunction instead. * UniversalFallback now returns a copy of the objective function it stores for ObjectiveFunction. * Use copy_if_modifiable instead of copy.
Bridges storing the function of the constraint they bridge now only implement UnsafeConstraintFunction and the copy is done by a fallback for ConstraintFunction in bridge.jl. Bridges building a new function only implement ConstraintFunction and UnsafeConstraintFunction falls back to it. SplitIntervalBridge forwards UnsafeConstraintFunction to its inner constraints. The bridge optimizer forwards UnsafeConstraintFunction to the inner model and to the bridges. UnsafeConstraintFunction has the same value as ConstraintFunction except that it may not be an independent copy so MockOptimizer now maps its indices as for ConstraintFunction.
* The fallbacks of ConstraintFunction and UnsafeConstraintFunction for
bridges calling each other created an infinite recursion for bridges
implementing neither of them, which is supported with
cannot_unbridge = true. The bridge optimizer now gets ConstraintFunction
from the bridges and UnsafeConstraintFunction throws the same
GetAttributeNotAllowed{ConstraintFunction} as before for bridges
implementing neither.
* Fix ambiguities of the ConstraintFunction fallback for bridges with the
getters of Variable.SetMapBridge and AbstractFunctionConversionBridge.
* The fallback of UnsafeObjectiveFunction is now a get_fallback as it was
ambiguous with the getters of model attributes of wrappers such as
MockOptimizer and CachingOptimizer. The bridge optimizer implements
UnsafeObjectiveFunction with ObjectiveFunction as the objective function
may be bridged.
Replace the unreachable ConstraintFunction model-attribute method with an ObjectiveFunction method that reads UnsafeObjectiveFunction while setting the regular objective attribute. Test affine, quadratic, and nonlinear objectives with identity and permuted index maps, including independent storage and modification for modifiable functions. Validation: all nine test files changed by the PR pass (138 test groups); temporary line markers confirm the new path executes for all three function types. JuliaFormatter 2.14.7 leaves no formatting diff.
It seems wasteful to copy the functions in
copy_tobefore it goes through theIndexMapwhen we also copy the function when it arrives in the model because we don't allow aMOI.ModelLiketo keep a copy of the function.For most implementation of
MOI.ModelLike,MOI.ConstraintFunctiondoes create a new copy but forMOI.Utilities.VectorOfConstraints, it is actually making a copy.What's a bit inconsistent is that
MOI.Utilities.ObjectiveContainerdoes not create a copy of the objective function at the moment. With this PR, the way we handle objective and constraints are not consistent.Removes the need for #2803
Closes #2803