Add support for min and max operators in GenericNonlinearExpr

[odow]

Closes #3508

[codecov]

Codecov Report

Patch and project coverage have no change.

Comparison is base (0c53e0b) 98.14% compared to head (6f3468e) 98.14%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3509   +/-   ##
=======================================
  Coverage   98.14%   98.14%           
=======================================
  Files          37       37           
  Lines        5545     5545           
=======================================
  Hits         5442     5442           
  Misses        103      103           

Files Changed	Coverage Δ
src/nlp_expr.jl	99.51% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[davidanthoff]

Wow, that was fast, thanks a lot :)

[odow]

I'm about to tag v1.15, so I wanted it in

[davidanthoff]

@odow I just ran into a corner case that is not yet handled by this PR: min and max can also be called with just one argument, and the current implementation doesn't solve that.

For now I can work around this by adding

# TODO Remove this once this is fixed in JuMP
function Base.min(x::JuMP.AbstractJuMPScalar)
    JuMP._throw_if_not_real(x)
    return JuMP.GenericNonlinearExpr{JuMP.variable_ref_type(x)}(:min, x)
end

to our own codebase, but I think this should really live here in JuMP. I would add a PR, but not entirely sure how you want to handle this kind of thing as you seem to use macros a bit to add these permutations, right?

[odow]

Argh. Why would min and max be unary methods 😢 Let me take a look

[davidanthoff]

Yeah, in MPSGE.jl we ran into this case because these expressions get auto-generated. We could work around it on our end, but given that Base.min and Base.max accept one arg as well, it seems better to fix here, right?

[odow]

I think this is better

Base.min(x::JuMP.AbstractJuMPScalar) = x
Base.max(x::JuMP.AbstractJuMPScalar) = x