ParametricOptInterface.jl icon indicating copy to clipboard operation
ParametricOptInterface.jl copied to clipboard

[breaking] drop invalid nonlinear support

Open odow opened this issue 1 year ago • 3 comments

Closes https://github.com/jump-dev/ParametricOptInterface.jl/issues/146

We actually don't support the NLPBlock because POI re-orders the variables by removing parameters. The tests passed because they had parameters at the end. But that isn't a good solution, and it seems simpler to drop support for the legacy nonlinear interface than throw an error if parameters are not in the right order.

We also don't really need POI for nonlinear. Ipopt supports parameters directly.

odow avatar Mar 01 '24 02:03 odow

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.92%. Comparing base (4ec565a) to head (ec7ed41).

:exclamation: Current head ec7ed41 differs from pull request most recent head 343f7b3

Please upload reports for the commit 343f7b3 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #147      +/-   ##
==========================================
- Coverage   94.96%   94.92%   -0.04%     
==========================================
  Files           5        5              
  Lines        1032     1025       -7     
==========================================
- Hits          980      973       -7     
  Misses         52       52              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Mar 01 '24 03:03 codecov[bot]

1 - Is there a tutorial using NLP parameters? 2 - In theory, we could preprocess the new nonlinear expression for VariableRef-in-MOI.Parameter, but it might be messy, correct?

joaquimg avatar Mar 01 '24 03:03 joaquimg

  1. If you're using Ipopt, you don't need POI. Syntax is the same.
  2. Yes, we could support MOI.ScalarNonlinearFunction. The issue is that we can't rewrite NLPBlock.

odow avatar Mar 01 '24 03:03 odow

Bump. I get that this is breaking, but the previous approach was incorrect and can't be worked around.

odow avatar Jun 24 '24 23:06 odow

Lets merge this

joaquimg avatar Jun 25 '24 00:06 joaquimg