Skip to content

Allow changing default cache for instantiate#1381

Merged
odow merged 2 commits intomasterfrom
bl/default_cache
Feb 15, 2022
Merged

Allow changing default cache for instantiate#1381
odow merged 2 commits intomasterfrom
bl/default_cache

Conversation

@blegat
Copy link
Copy Markdown
Member

@blegat blegat commented Jun 3, 2021

The main use case for this is that once a solver defines a fast copy_to method with a MatrixOfConstraint model, then it can use it as cache directly to avoid having to store an extra cache. As instantiate is used by JuMP, this will be what's used by default in JuMP.

Copy link
Copy Markdown
Member

@odow odow left a comment

Choose a reason for hiding this comment

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

This is really only useful for the cache inside a bridge, correct?

But this seems like you want JuMP to go with Option 2 as discussed here: jump-dev/Clp.jl#114 (comment).

This has some pretty big implications that I don't fully understand. Like it's now not possible to modify a cache after copy_to has been called.

@blegat
Copy link
Copy Markdown
Member Author

blegat commented Jun 3, 2021

Like it's now not possible to modify a cache after copy_to has been called.

Yes, it is breaking. However, this cache is mostly there because the bridges need the underlying model to support incremental loading of the model, not to support modification if the solver don't support it.

If the user want to be sure to be able to do modification MOI.instantiate(optimizer_constructor, with_bridge_type=T) is not the answer. Indeed, if the solver implement incremental loading but not modifications then no cache is added and it will fail when modifying.
So a solver-independent code can assume that MOI.instantiate(optimizer_constructor, with_bridge_type=T) supports incremental loading but not that it can support modifications after copy_to is called. This was already true before this PR.
To make incremental loading work without copy_to, we might need a few tweaks though such that calling MOIU.final_touch in CachingOptimizer before attempting to synchronize the optimizer.

@blegat blegat force-pushed the bl/default_cache branch from 6d072db to 2611f43 Compare June 15, 2021 22:38
@odow
Copy link
Copy Markdown
Member

odow commented Jun 15, 2021

Before merging, I'd like to see this implemented by a solver.

@blegat
Copy link
Copy Markdown
Member Author

blegat commented Jun 17, 2021

I was doing it for Clp and then I got hit by issues which lead me to #1386 and #1387. I'll continue when these two are done.

@odow odow merged commit 5e4cd36 into master Feb 15, 2022
@odow odow deleted the bl/default_cache branch February 15, 2022 03:07
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.

2 participants