-
Notifications
You must be signed in to change notification settings - Fork 92
Remove precompile statements #2643
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
Conversation
a1a9bff
to
7f8dd1c
Compare
JuMP and HiGHS both have precompile statements so that's a bit misleading. Maybe also include benchmarks with pure-MOI and a solver that does not have precompile statemtents |
) | ||
MOI.precompile_model(UniversalFallback{Model{T}}, constraints) | ||
MOI.precompile_model( | ||
CachingOptimizer{MOI.AbstractOptimizer,UniversalFallback{Model{T}}}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@blegat: we don't build this model anymore in JuMP, so these weren't actually being used.
Arguably, this could make TTFS slower for new versions of MOI and an old version of Julia like 1.6. For other solvers: we should improve them (if needed) by adding PrecompileTools to their packages. Not to MOI. I don't think any package was actually using |
Here's SCS: Old
New
There's no I guess the action item in addition to this PR is to add precompilation things to each solver package. |
7f8dd1c
to
5903b12
Compare
I don't think we should be worried about users not using the LTS.
Yes, I just thought it would be useful to get a sense of whether users would notice a regression in the meantime. Looking at your SCS benchmark, it seems fine. |
These were once necessary, but now they're doing more harm than good.
The
precompile_model
functions are safe to replace with a warning because they have no user-visible effect.Code
Before
This PR
There are also now better tools like PrecompileTools.jl so that we can actually build and solve problems in each package if needed.