Add @nonlinear macro for modifying how expressions are parsed

[odow]

x-ref https://github.com/jump-dev/JuMP.jl/issues/3498 x-ref https://github.com/jump-dev/JuMP.jl/issues/3729

• [x] I'm open to bikeshedding the name (and whether we should even add this).

A core problem is that @mitchphillipson's models from #3729 contain (x * (1 + y)) / c, which creates an AffExpr 1 + y, a QuadExpr x * (1 + y), and then a new QuadExpr (x * (1 + y)) / 2 (because /(::Quad, ::Real) is not in-place.

Since everything is about to be converted to NonlinearExpr anyway, it'd be nicer if we just kept things as NonlinearExpr.

cc @Downsite

[blegat]

Since everything is about to be converted to NonlinearExpr anyway, it'd be nicer if we just kept things as NonlinearExpr.

In this case yes, but this performance issue could be hit in cases where a quadratic expression is the desired typed. If the rewriting could would rewrite (x * (1 + y)) / 2 into MA.operate!!(/, x * (1 + y), 2) then the performance issue would be fixed (provided the corresponding methods are defined in MA). I'd prefer fixing the performance issue, especially since it falls exactly within realms of issues MA was designed to fix.

[odow]

but this performance issue could be hit in cases where a quadratic expression is the desired typed.

Exactly. Which is why this @nl is opt-in. We want to default to building affine and quadratic expressions. But sometimes people might want nonlinear ones.

It's not really about this specific performance issue. Even if operate!(/, Float64, lhs, rhs) was implemented, there would still be more temporaries allocated because creating AffExpr and QuadExpr require allocating OrderedDict which are expensive.

[blegat]

I don't see any objection then, I can see this being useful. About the naming, once we drop the legacy nonlinear interface, what would be left of the nl shortcut ? If not much then maybe the nl shortcut is not so standard anymore in JuMP and we can use @nonlinear

[odow]

Yeah I have no strong opinions on the name. I don't like @nl(...). @nonlinear could work.

[odow]

Bikeshed

• @_nl ?
• @nl_esc
• @scary_nl
• @nl_boo (@matbesancon)
• @nlexpr(...)
• @nl_expr(...)
• @raw_nl_expr(...)
• @raw_nonlinear_expr(...)
• @raw_nl
• @nl_rewrite
• @force_nl
• @force_nl_expr
• @force_nonlinear
• ...

If we use @force_nonlinear, then we should assert that the return is a GenericNonlinearExpr.

The error would catch things like:

f(x) = x^2
@force_nonlinear(f(x)) :: QuadExpr

where the @force_nonlinear does not actually return a GenericNonlinearExpr.

[codecov[bot]]

Codecov Report

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

Project coverage is 98.40%. Comparing base (617f961) to head (9ab125e). Report is 11 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3732      +/-   ##
==========================================
- Coverage   98.42%   98.40%   -0.02%     
==========================================
  Files          43       44       +1     
  Lines        5825     5842      +17     
==========================================
+ Hits         5733     5749      +16     
- Misses         92       93       +1     

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

[odow]

This could use a review. I've updated to @force_nonlinear as discussed.

[Downsite]

@odow Is it expected that @force_nonlinear does not work when the quadratic expression comes from a user-defined function?

@force_nonlinear(p.funcs.g(x, w[j, :])): expression did not produce a GenericNonlinearExpr. Got a QuadExpr: w[1,1]*w[1,2] - x[1]*w[1,2] + w[1,1] + 0 x[1]")

[odow]

Yes, because the macro cannot rewrite the expressions inside p.funcs.g.

We considered automatically converting the result, but decided against it.

You should modify p.funcs.g to create the appropriate expression type.