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

Cache whether solution has a FEASIBLE_POINT #178

Merged

Conversation

thiagonovaesb
Copy link

@thiagonovaesb thiagonovaesb commented Sep 2, 2022

Querying solution values involves many checks to see if the attribute like VariablePrimal(result_index) is within the number of available results. This requires a call to get the PrimalStatus, which can be expensive. This PR adds a cache for the primal status to improve things.

add has_feasible_point flag
@@ -229,7 +231,7 @@ mutable struct Optimizer <: MOI.AbstractOptimizer

# TODO: add functionality to the lower-level API to support querying single
# elements of the solution.

optimizer_called::Bool
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't need to add this flag. It's okay for the optimizer to return a previous solution, even if the problem was modified. (JuMP handles these checks so the solvers don't need to: https://jump.dev/JuMP.jl/stable/manual/solutions/#OptimizeNotCalled-errors.)

The test you found was a bit weird, but it wasn't necessarily wrong. Is there a larger problem than the test that you found in your own code?

Copy link
Author

@thiagonovaesb thiagonovaesb Sep 2, 2022

Choose a reason for hiding this comment

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

I think I had some errors because I was using Xpress 8.5 and ended up adding unnecessary things.

I am solving multiple problems in sequence. But every time I query variable values Xpress has to go through MOI.get(model, MOI.PrimalStatus()), which has become a bottleneck.

I would just like to create a cache so I don't have to call this function so many times. I removed all the lines about optimizer_called and left only the has_feasible_point cache.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, that makes sense. Can you post a screenshot of the tests passing? You don't have permission to run the CI under your account.

@odow odow changed the title add optimizer called flag Cache whether solution has a FEASIBLE_POINT Sep 2, 2022
@odow odow requested a review from joaquimg September 2, 2022 04:15
@thiagonovaesb thiagonovaesb deleted the tn/add_optimizer_called_flag branch September 2, 2022 04:23
@thiagonovaesb thiagonovaesb restored the tn/add_optimizer_called_flag branch September 2, 2022 04:32
@odow
Copy link
Member

odow commented Sep 2, 2022

Why'd you close? 😆

@thiagonovaesb
Copy link
Author

thiagonovaesb commented Sep 2, 2022

@odow Sorry, I thought about renaming my branch, because the current name doesn't make sense anymore. But that closed the PR automatically.

@thiagonovaesb thiagonovaesb reopened this Sep 2, 2022
@thiagonovaesb
Copy link
Author

image
image
image
image
image

@joaquimg joaquimg merged commit 336973f into jump-dev:master Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants