DNMY: [FileFormats.MOF] change parse_as_nlpblock to false by default #2295
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.
I've opened this PR, not because I want to merge it, but because I think it needs some discussion.
Currently, if you have a
.mof.json
file that contains nonlinear functions, they are parsed into anNLPBlock
.#2293 added support for parsing them as regular
ScalarNonlinearFunction
s, but made it opt-in behind aparse_as_nlpblock
keyword.To read in JuMP, you'd need to write:
This PR proposes swapping the default, which is a breaking change if people expect the
NLPBlock
to exist, or if they're passing to a solver that doesn't support the new syntax yet.We could potentially reduce the breakage by changing
default_copy_to
, so that, if asrc
model hasScalarNonlinearFunction
but thedest
model doesn't support them, we could convert to aNLPBlock
. This would then only impact people usingMOF.Model
directly, not not some reading from a file in JuMP.Or we could say that the nonlinear support in
.mof.json
was always experimental. It seems likely that very few people are actually using.mof.json
files to store models, particularly nonlinear ones.Or we could keep the default as-is and add a warning telling people to use the keyword argument, and that it might change the default in a future release.
Thoughts?