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

Update to MathOptInterface v1.0 #218

Merged
merged 5 commits into from
Mar 24, 2022
Merged

Update to MathOptInterface v1.0 #218

merged 5 commits into from
Mar 24, 2022

Conversation

odow
Copy link
Contributor

@odow odow commented Mar 21, 2022

How does requires work if the requires package introduces breaking changes? I found: JuliaPackaging/Requires.jl#82

It seems like GalacticOptim has some strong compatibility issue with MathOptInterface, and yet there's nothing in the Project.toml to prevent someone installing incompatible versions.

Closes #214
Closes #167

@ChrisRackauckas
Copy link
Member

How does requires work if the requires package introduces breaking changes? I found: JuliaPackaging/Requires.jl#82

We need to fix this by moving to subpackages. Requires.jl indeed does not play nicely with compatibilities.

@odow
Copy link
Contributor Author

odow commented Mar 21, 2022

Ah. This is going to need the Nonconvex packages updated. cc @mohamed82008

src/solve/moi.jl Outdated Show resolved Hide resolved
@odow
Copy link
Contributor Author

odow commented Mar 21, 2022

@odow
Copy link
Contributor Author

odow commented Mar 22, 2022

Let's hold off merging this for a bit. I'm looking into a bug that @ccoffrin found.

src/solve/moi.jl Outdated Show resolved Hide resolved
src/solve/moi.jl Show resolved Hide resolved
src/solve/moi.jl Show resolved Hide resolved
@odow
Copy link
Contributor Author

odow commented Mar 22, 2022

Now blocked by:

odow referenced this pull request in lanl-ansi/rosetta-opf Mar 22, 2022
@ChrisRackauckas
Copy link
Member

@blegat and @jonasmac16 if you want to take a look.

@ccoffrin
Copy link
Contributor

@odow, looks like Tulip issue was resolved. Is this branch passing tests now?

@odow odow closed this Mar 24, 2022
@odow odow reopened this Mar 24, 2022
@odow
Copy link
Contributor Author

odow commented Mar 24, 2022

I give up. There are still a bunch of compatibility issues with the Nonconvex stuff. If we want to test a bunch of different packages like this, it should be done in separate projects. There's no need to attempt to install everything in a single environment.

And the tests don't even pass on master anyway:
https://github.com/SciML/GalacticOptim.jl/runs/5632625030?check_suite_focus=true

@ccoffrin
Copy link
Contributor

ccoffrin commented Mar 24, 2022

FWIW, odow:od/moi1 is working for me and hence is an improvement over master. I support merging even if all tests do not pass.

@Vaibhavdixit02
Copy link
Member

@mohamed82008 would you be able to help out with the version issues?

@Vaibhavdixit02
Copy link
Member

I support merging even if all tests do not pass.

Let's wait a couple of days and see if we can get some help to get the version conflicts resolved, otherwise we'll go ahead and merge this as is, hope that's ok?

@ChrisRackauckas
Copy link
Member

We plan to split the wrappers out to separate subpackages, like how KernelAbstractions.jl is done with the lib being completely different repos. See https://github.com/JuliaGPU/KernelAbstractions.jl/tree/master/lib. This would then get rid of Requires.jl usage entirely (so good for startup times), and would make it easier to version each subpackage separately. The individual subpackages can then be ran as part of the GalacticOptim.jl tests, but be able to participate in version control (since Requires.jl has a pretty bad issue with that 😅).

Our first experiment with doing this is NeuralPDELogging.jl JuliaRegistries/General#57196. It seems like that's working now, so we should do this here.

@ChrisRackauckas
Copy link
Member

Let's wait a couple of days and see if we can get some help to get the version conflicts resolved, otherwise we'll go ahead and merge this as is, hope that's ok?

I'm just going to merge so I can do a follow-up PR that splits to version controlled subpackages and fix this madness.

@ChrisRackauckas ChrisRackauckas merged commit ea677e1 into SciML:master Mar 24, 2022
@mohamed82008
Copy link

I'm just going to merge so I can do a follow-up PR that splits to version controlled subpackages and fix this madness.

This madness is one of the motivations behind the Nonconvex.jl design of a shell package that loads algorithms from their respective packages.

@jonasmac16
Copy link
Contributor

Let's wait a couple of days and see if we can get some help to get the version conflicts resolved, otherwise we'll go ahead and merge this as is, hope that's ok?

I'm just going to merge so I can do a follow-up PR that splits to version controlled subpackages and fix this madness.

Sorry, I didn't have time to look at the PR (in the last 2 weeks of my PhD atm :( ). Happy to help with the subpackages once that is done as this has been on my todo list for some time.

@ChrisRackauckas
Copy link
Member

Note that we moved everything to a subpackage form, so now all of the solvers track their own dependencies in their Project.toml. We need to figure out what's going on with Nonconvex.jl's bindings, but all of the other ones should be good now.

@odow odow deleted the od/moi1 branch May 20, 2022 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compatibility with MOI v1 MOI throws error with EAGO.jl
6 participants