[Nonlinear] add new submodule

[odow]

At attempt to make #1803 more reviewable.

TODO before merging:

• Relicensing
  • @amontoison

[odow]

I discussed this with @matbesancon, and we decided that that easiest way forward is to make a new NonlinearOptInterface package, to merge this code into that, and then for JuMP to (temporarily) depend upon NonlinearOptInterface.jl.

That lets us break things at will, and we can make small PRs without having to manage this stack (GitHub should make it easier to manage stacked PRs).

[mlubin]

I discussed this with @matbesancon, and we decided that that easiest way forward is to make a new NonlinearOptInterface package, to merge this code into that, and then for JuMP to (temporarily) depend upon NonlinearOptInterface.jl.

That lets us break things at will, and we can make small PRs without having to manage this stack (GitHub should make it easier to manage stacked PRs).

Wouldn't working on branches of MOI and JuMP accomplish the same goal? You can submit PRs against branches.

[matbesancon]

Wouldn't working on branches of MOI and JuMP accomplish the same goal? You can submit PRs against branches.

It can be a bit harder to keep track of a separate branch all the time. A separate repo can also work better to dev the package but still keep MOI as-is, seeing the current state, etc

[odow]

Okay, I've done quite a large refactoring. We now have Model, Evaluator and NLPBlockData:

model = Nonlinear.Model()
# add expressions, constraints etc.
evaluator = Nonlinear.Evaluator(model, Nonlinear.ExprGraphOnly(), [x])
block = MOI.NLPBlockData(evaluator)

I think that makes the distinction a lot clearer, because Nonlinear.Model is now just the data structure, with no fields for any AD. Presumably, someone could also write a presolve(::Model)::Model function, which consumes a model and returns a presolved model.

[mlubin]

The refactor is a big improvement.

Ok to merge or split off into a package (as you prefer). IMO it should be manageable to keep working within the MOI package using branches as needed.

[odow]

Ok to merge...IMO it should be manageable to keep working within the MOI package using branches as needed.

You mean merge into MOI proper? Or squash these commits and then leave it as a long-running branch that we merge other PRs into?

[mlubin]

I don't see the harm in merging into MOI master.

[odow]

Ah okay. I guess this warning is sufficient for the near term:

[matbesancon]

The other argument for having this in a separate package is keeping compilation time at a reasonable level, since one might change this one often in some cases without touching the rest of MOI. Having separate packages means separate compilation units

[blegat]

Trying to see what this would give in the future, I guess we could have

function Nonlinear.set_objective(model, expr)
    func = Nonlinear.parse_expression(model, expr)
    MOI.set(model, MOI.ObjectiveFunction{typeof(obj)}(), func)
end
function Nonlinear.add_constraint(model, expr, set)
    expr = parse_expression(model, expr)
    MOI.add_constraint(model, func, set)
end

so these functions would provide a convenience API for providing expressions with the :(...) syntax.
Is that aligned with what you imagine ? If we want to be able to break Nonlinear in the future, should we name it _Nonlinear ?

[matbesancon]

Is that aligned with what you imagine ? If we want to be able to break Nonlinear in the future, should we name it _Nonlinear

There is a warning at the top-level of the Nonlinear doc that anything in there is experimental and subject to breaking changes.

We could add a statement in a docstring attached to the Nonlinear submodule itself

[blegat]

Doc is helping but I don't know if that's sufficient, might be though, just think we should consider both possibilities.

[odow]

We could add a statement in a docstring attached to the Nonlinear submodule itself

I'll add a docstring. No one is going to be messing with this submodule accidentally. The docs are pretty clear that it is experimental, so I don't mind breaking things at-will for the near-term.