Remove NLP block support, add ScalarNonlinearFunction support

[chriscoey]

replaces #23. uses MOI ScalarNonlinearFunction CC @odow

[odow]

To get CI passing, in test/runtests.jl, add:

import Pkg
Pkg.pkg"add MathOptInterface#od/nlp-expr

You'll also need to add Pkg as a dependency to Project.toml.

[codecov[bot]]

Codecov Report

Patch coverage: 99.06% and project coverage change: +1.02% :tada:

Comparison is base (ec190bb) 95.92% compared to head (6293602) 96.95%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #25 +/- ## ========================================== + Coverage 95.92% 96.95% +1.02% ========================================== Files 3 3 Lines 442 427 -15 ========================================== - Hits 424 414 -10 + Misses 18 13 -5 ``` | [Files Changed](https://app.codecov.io/gh/jump-dev/MiniZinc.jl/pull/25?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jump-dev) | Coverage Δ | | |---|---|---| | [src/write.jl](https://app.codecov.io/gh/jump-dev/MiniZinc.jl/pull/25?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jump-dev#diff-c3JjL3dyaXRlLmps) | `99.35% <99.05%> (+0.60%)` | :arrow_up: | | [src/MiniZinc.jl](https://app.codecov.io/gh/jump-dev/MiniZinc.jl/pull/25?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jump-dev#diff-c3JjL01pbmlaaW5jLmps) | `100.00% <100.00%> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/jump-dev/MiniZinc.jl/pull/25/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jump-dev)

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

[chriscoey]

@odow there's a failing test with the new vector argument SNF. MOI doesn't like MOI.ScalarNonlinearFunction(:∀, Any[[snf1, snf2]]) I think:

test_model_nonlinear_bool_vector_arg: Error During Test at /Users/ccoey/.julia/dev/MiniZinc/test/runtests.jl:23
  Got exception outside of a @test
  MethodError: no method matching map_indices(::MathOptInterface.Utilities.var"#1#2"{MathOptInterface.Utilities.IndexMap}, ::Vector{MathOptInterface.ScalarNonlinearFunction})

  Closest candidates are:
    map_indices(::Any, ::MathOptInterface.RawOptimizerAttribute, ::Any)
     @ MathOptInterface ~/.julia/packages/MathOptInterface/Fqsnb/src/Utilities/functions.jl:92
    map_indices(::Any, ::Union{MathOptInterface.AbstractConstraintAttribute, MathOptInterface.AbstractModelAttribute, MathOptInterface.AbstractOptimizerAttribute, MathOptInterface.AbstractVariableAttribute}, ::Any)
     @ MathOptInterface ~/.julia/packages/MathOptInterface/Fqsnb/src/Utilities/functions.jl:89
    map_indices(::F, ::Union{MathOptInterface.ScalarQuadraticFunction, MathOptInterface.VectorQuadraticFunction}) where F<:Function
     @ MathOptInterface ~/.julia/packages/MathOptInterface/Fqsnb/src/Utilities/functions.jl:200
    ...

  Stacktrace:
    [1] map_indices(index_map::MathOptInterface.Utilities.var"#1#2"{MathOptInterface.Utilities.IndexMap}, f::MathOptInterface.ScalarNonlinearFunction)
      @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/Fqsnb/src/Utilities/functions.jl:213
    [2] map_indices
      @ ~/.julia/packages/MathOptInterface/6V5dl/src/Utilities/functions.jl:106 [inlined]
    [3] _copy_constraints(dest::MiniZinc.ModelFunctionConstraints{Int64}, src::MathOptInterface.Utilities.Model{Int64}, index_map::MathOptInterface.Utilities.IndexMap, index_map_FS::MathOptInterface.Utilities.DoubleDicts.IndexDoubleDictInner{MathOptInterface.ScalarNonlinearFunction, MathOptInterface.EqualTo{Int64}}, cis_src::Vector{MathOptInterface.ConstraintIndex{MathOptInterface.ScalarNonlinearFunction, MathOptInterface.EqualTo{Int64}}})

[odow]

We probably need a method like

function MOI.Utilties.map_indices(::F, x::AbstractVector{T}) where {F,T}
    return [MOI.Utilties.map_indices(F, xi) for xi in x]
end

There's an entire universe of methods in MOI.Utilities that are implemented only because at some point we hit a MethodError.

[odow]

Doesn't it make more sense to be MOI.ScalarNonlinearFunction(:∀, Any[snf1, snf2])? I don't see how ∀ is univariate.

[chriscoey]

In MiniZinc forall, exists, count etc operate on vector arguments. see e.g. https://www.minizinc.org/doc-2.7.0/en/lib-stdlib-builtins.html?highlight=xorall#mzn-ref-stdlib-builtins-logic-exists

note "clause" above operates on two vectors.

predicate clause(array [$T] of var bool: x, array [$T] of var bool: y)

to support clause we would need to do SNF(:clause, Any[vec1, vec2]), where vec1 and vec2 are vectors. forall, exists etc are just done in a way that is consistent with minizinc here.

[chriscoey]

it seems consistent with Julia too. i mean you count a vector of booleans, meaning the argument to count is a vector and count is unary.

julia> count(true, false, true)
ERROR: MethodError: no method matching count(::Bool, ::Bool, ::Bool)

julia> count([true, false, true])
2

similar argument for forall, exists

[odow]

I guess following Base is reasonable. I feel like at some point we're going to need to standardize all of these, otherwise we're going to run into issues.

The things for which there isn't a good Julia equivalent like clause are much harder to know what to do with.

[chriscoey]

well clause is like, say +(vec1, vec2). what's the problem with SNF(:clause, Any[vec1, vec2])?

[chriscoey]

if you can add the missing methods and test e.g. +(vec1, vec2) and count(vec) as SNF tests to MOI, then this test should pass here just fine and the code is very clean here at least. is the problem that you think we should wait until we have a VectorNonlinearFunction type?

[odow]

The problem with clause is that it doesn't have a nice Julia equivalent.

If I understand https://www.minizinc.org/doc-2.6.4/en/lib-stdlib-builtins.html?highlight=clause#mzn-stdlib-builtins-logic-clause correctly, we could define a bunch of functions in MOI, like MOI.clause(x, y) = any(x) || any(!, y).

But otherwise there's no good way to actually evaluate these things. It'd turn into a symbolic free-for-all of hoping that the solver understands specific things, and that isn't very extensible.

[odow]

One problem I have with constraint programming is that there are too many primitives to deal with. MIP/Conic is nice in that small pieces make a bigger problem.

[odow]

I guess there's a trade-off between MOI being very rigid about what atoms it supports, and being very flexible in allowing the solver to choose.

The rigid way would let us write bridges/transformations, e.g., from nonlinear to conic. The latter would end up with users writing solver-specific models, in which to use MiniZinc you need to use mzn_all(x) and with SCIP you need to use scp_all(x).

But I don't think that discussion is a blocker for the ScalarNonlinearFunction PR in MOI. My takeaway from this is that it seems to work, maybe with a few additional tests and some missing methods?

We can talk in tomorrow's nonlinear call

[odow]

I hopefully fixed the map_indices problem with the latest commit to MOI

[chriscoey]

I guess only question is why remove NLPBlock support instead of just making this a feature addition?

I think we discussed that early on but decided not to. It is more legacy code to maintain in a package/interface that is fairly experimental. Makes it harder to extend in future and it's not the future of nonlinear modeling.

[chriscoey]

thanks @odow. needs MOI v1.19

[chriscoey]

Hey @odow - just requesting a review on this. Are there any relevant MOI changes coming down the pike?