Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bridges] add ScalarQuadraticToScalarNonlinearBridge #2233

Merged
merged 3 commits into from
Jun 29, 2023
Merged

Conversation

odow
Copy link
Member

@odow odow commented Jun 29, 2023

cc @chriscoey

Necessary precursor to jump-dev/Pavito.jl#67

Basic

  • Create a new file in src/Bridges/XXX/bridges
  • Define the bridge, following existing examples. The name of the bridge
    struct must end in Bridge
  • Check if your bridge can be a subtype of MOI.Bridges.Constraint.SetMapBridge
  • Define a new const that is a SingleBridgeOptimizer wrapping the
    new bridge. The name of the const must be the name of the bridge, less
    the Bridge suffix
  • include the file in src/Bridges/XXX/bridges/XXX.jl
  • If the bridge should be enabled by default, add the bridge to
    add_all_bridges at the bottom of src/Bridges/XXX/XXX.jl

Tests

  • Create a new file in the appropriate subdirectory of tests/Bridges/XXX
  • Use MOI.Bridges.runtests to test various inputs and outputs of the
    bridge
  • If, after opening the pull request to add the bridge, some lines are not
    covered by the tests, add additional bridge-specific tests to cover the
    untested lines.

Documentation

  • Add a docstring which uses the same template as existing bridges.
  • Add the docstring to docs/src/submodules/Bridges/list_of_bridges.md

Final touch

If the bridge depends on run-time values of other variables and constraints in
the model:

  • Implement MOI.Utilities.needs_final_touch(::Bridge)
  • Implement MOI.Utilities.final_touch(::Bridge, ::MOI.ModelLike)
  • Ensure that final_touch can be called multiple times in a row

return ScalarQuadraticToScalarNonlinearBridge{T,S}
end

function MOI.get(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method and the next two should be implemented at the AbstractFunctionConversion level

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this can be done in a separate PR

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah there's the canonical issue as well. But we can come back to it with #2224

@odow
Copy link
Member Author

odow commented Jun 29, 2023

I just removed the canonical method. It'll get added back generically by #2224

@chriscoey
Copy link
Contributor

thanks @odow! exciting.
here's the code I was using to convert SNF to SAF/SQF previously (which may not work in future if operate supports SNF?):

# convert SNF to SAF/SQF{T}
function nl_to_aff_or_quad(::Type{T}, f::MOI.ScalarNonlinearFunction) where {T <: Real}
    args = nl_to_aff_or_quad.(T, f.args)
    if !any(Base.Fix2(isa, MOI.ScalarNonlinearFunction), args)
        if f.head == :^
            if length(args) == 2 && args[2] == 2
                return MOI.Utilities.operate(*, T, args[1], args[1])
            end
        else
            h = get(_quad_ops, f.head, nothing)
            isnothing(h) || return MOI.Utilities.operate(h, T, args...)
        end
    end
    return error("function $f cannot be converted to linear or quadratic form")
end

nl_to_aff_or_quad(::Type{<:Real}, f::MOI.VariableIndex) = f
nl_to_aff_or_quad(::Type{T}, f::T) where {T <: Real} = f
nl_to_aff_or_quad(::Type{T}, f::Real) where {T <: Real} = convert(T, f)

_quad_ops = Dict(:+ => +, :- => -, :* => *, :/ => /)

do you know off-hand if the SNF to SAF/SQF convert in this PR will handle all the cases the above code does? I'd love to be able to remove my hacky code when this PR lands.

@odow
Copy link
Member Author

odow commented Jun 29, 2023

Yeah that code will now just make a ScalarNonlinearFunction. Let me take another look.

@odow
Copy link
Member Author

odow commented Jun 29, 2023

So I guess one decision is how hard we want to fight to recognize *(2, *(3, /(x, 2))) = 2 * (3 * x / 2) = 3x as an affine expression. Also things like +(x, sin(1.0)). Should we see :sin and error? Or recognize that it's actually a constant.

Perhaps this would be easier if we had a smart canonicalize function which grouped like terms and lifted nested + and * etc.

@odow
Copy link
Member Author

odow commented Jun 29, 2023

That'll do for now. We can revisit convert is a separate PR if needed.

@odow odow merged commit 5ac9cbb into master Jun 29, 2023
@odow odow deleted the od/sqf-to-snf branch June 29, 2023 22:13
@odow odow added the Project: next-gen nonlinear support Issues relating to nonlinear support label Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Project: next-gen nonlinear support Issues relating to nonlinear support
Development

Successfully merging this pull request may close these issues.

3 participants