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

Add [Nonlinear.ReverseAD] submodule #1803

Merged
merged 16 commits into from
May 5, 2022
Merged

Add [Nonlinear.ReverseAD] submodule #1803

merged 16 commits into from
May 5, 2022

Conversation

odow
Copy link
Member

@odow odow commented Apr 18, 2022

Testing this as a submodule of MOI to see what the impact would be on JuMP

Docs: https://jump.dev/MathOptInterface.jl/previews/PR1803/submodules/Nonlinear/overview/

Before merging:

  • Rename to _Nonlinear, or otherwise indicate that the API is experimental and subject to change. Added a warning to the docs.

The majority of this development was carried out in the JuMP PRs:

Nonlinear.ReverseAD is a minor refactoring of code that previously
existed in JuMP under the _Derivatives submodule, and prior to that
the ReverseDiffSparse.jl package.

Once merged open issues for:

@odow odow changed the title DNMY: [Nonlinear] add the nonlinear submodule RFC: [Nonlinear] add the nonlinear submodule Apr 19, 2022
@odow odow added the Project: next-gen nonlinear support Issues relating to nonlinear support label Apr 19, 2022
@mlubin
Copy link
Member

mlubin commented Apr 19, 2022

This is too big to review. One possible split is 1) introduce the nonlinear data structures into MOI 2) add the ReverseAD backend

@matbesancon
Copy link
Contributor

Does this only introduce symbolic nonlinear and not oracle-based nonlinear?

@odow
Copy link
Member Author

odow commented Apr 19, 2022

This is too big to review.

Yeah. Now that it's in MOI, I'll break it back up again.

Does this only introduce symbolic nonlinear and not oracle-based nonlinear?

It is (just) a refactoring of JuMP's current nonlinear interface. But it's also designed to be extensible, so that SymbolicAD is now an add-on: lanl-ansi/MathOptSymbolicAD.jl#17

optimize!(model; differentiation_backend = SymbolicAD.DefaultBackend())
optimize!(model; differentiation_backend = MOI.Nonlinear.SparseReverseMode())

@matbesancon
Copy link
Contributor

matbesancon commented Apr 19, 2022

having to interpolate can make some friction, we should offer a substitute function and/or macro

@odow
Copy link
Member Author

odow commented Apr 19, 2022

having to interpolate can make some friction, we should offer a substitute function and/or macro

That's JuMP's job. It doesn't matter if this interface has some friction.

@odow odow mentioned this pull request Apr 19, 2022
1 task
@odow odow force-pushed the od/nonlinear branch 2 times, most recently from 2d301f5 to aabe99b Compare April 19, 2022 23:56
@odow odow changed the base branch from master to od/nonlinear-api April 19, 2022 23:56
@odow odow changed the title RFC: [Nonlinear] add the nonlinear submodule RFC: add [Nonlinear.ReverseAD] submodule Apr 19, 2022
@odow odow requested a review from mlubin April 28, 2022 18:07
odow added 10 commits May 3, 2022 08:44
The majority of this development was carried out in the JuMP PRs:
 * jump-dev/JuMP.jl#2939
 * jump-dev/JuMP.jl#2942
 * jump-dev/JuMP.jl#2943

Nonlinear.ReverseAD is a minor refactoring of code that previously
existed in JuMP under the _Derivatives submodule, and prior to that
the ReverseDiffSparse.jl package.
create a [`Nonlinear.Evaluator`](@ref) object with
[`Nonlinear.SparseReverseMode`](@ref), and then query the MOI API methods.

### Why another AD package?
Copy link
Member

Choose a reason for hiding this comment

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

I reject the premise of the question because this code was one of the first implementations of reverse-mode AD in Julia. This discussion as worded doesn't really fit as package documentation. What would fit is a discussion of the design goals of ReverseAD. It's fair to mention the history of the code as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

How's this now? I only made minor changes to split the history and identify design goals. But can make larger changes if you want?

Copy link
Member

@mlubin mlubin left a comment

Choose a reason for hiding this comment

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

Oops, I left a comment pending.

@odow odow merged commit 1ab5078 into master May 5, 2022
@odow odow deleted the od/nonlinear branch May 5, 2022 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants