Addition of copy_model and copy_gdp_data to copy GDP models.#126
Merged
pulsipher merged 8 commits intoinfiniteopt:masterfrom Nov 3, 2025
Merged
Addition of copy_model and copy_gdp_data to copy GDP models.#126pulsipher merged 8 commits intoinfiniteopt:masterfrom
pulsipher merged 8 commits intoinfiniteopt:masterfrom
Conversation
Contributor
Author
|
@pulsipher this is ready for review. |
Refactor variable creation and add copy_model_and_gdp_data function.
pulsipher
requested changes
Nov 3, 2025
Collaborator
pulsipher
left a comment
There was a problem hiding this comment.
Looking good, just some minor points to address
created more tests accordingly. addition of test to check that model instances do not affect each other.
pulsipher
requested changes
Nov 3, 2025
Collaborator
pulsipher
left a comment
There was a problem hiding this comment.
Looks good, just one more thing. Please add some comments make the code more readable. For example, a comment for each for loop in copy_gdp_data and some comments for the dispatch functions. Also, the new public functions should each have a docstring.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR extends
JuMP.copy_extension_datain order to allowGDPModels to be an input toJuMP.copy_model().JuMP.copy_extension_datadoes not receive a reference map as one of it's argument so it's impossible to recover disjunct constraints. Therefore another function was made to be called (DP.copy_gdp_data()) in order to populate the new model'sGDPData. The workflow goes sequentiallty as in the example below.In addition there is also
copy_model_and_gdp_datato do bothAn alternative would be to extend JuMP.copy_model itself but at a glance I believe there might be consequences when we want to have DP coexist with other packages and have the ability to copy.