Skip to content

pyomo.gdp: Fixing bugs in nested models in gdp.hull transformation#3143

Merged
emma58 merged 40 commits intoPyomo:mainfrom
emma58:fix-nested-hull
Feb 21, 2024
Merged

pyomo.gdp: Fixing bugs in nested models in gdp.hull transformation#3143
emma58 merged 40 commits intoPyomo:mainfrom
emma58:fix-nested-hull

Conversation

@emma58
Copy link
Copy Markdown
Contributor

@emma58 emma58 commented Feb 18, 2024

Fixes # .

Summary/Motivation:

This fixes several bugs in hull having to do with nested GDPs:

  1. Primarily, it corrects the prior implementation's wrong assumption that all nested indicator variables are local. This doesn't have to be true, and when it's not, these variables need to be disaggregated like anything else.
  2. It also corrects a bug in variable disaggregation in nested GDPs where some layers of the hierarchy don't use variables that appear as decendents to them in GDP trees.
  3. There was a silly typo bug having to do with disaggregation too.

Most notably, in order to accomplish the above, this switches the hull transformation from transforming from root to leaf in the GDP tree to the opposite. Note that this is the reason that the baselines changed and that the FME tests changed (and can be blamed for lots of other chaos as well).

Changes proposed in this PR:

  • Rewrites hull to transform from leaf to root in the GDP tree
  • Corrects handling of nested structures in terms of disaggregated variables, particularly not assuming any indicator variables are local unless it is declared
  • Adds/modifies a lot of tests for nested GDPs
  • Partially moves the mappings between original and transformed components to the new private_data magic, though does not do this completely yet--I will save that for a later PR

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the BSD license.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

emma58 added 29 commits November 1, 2023 11:45
…rmance improvements in the variable gathering logic
…ate some tests, fixing a bug in GDPTree.parent_disjunct method
…ed, simplifying how we deal with local vars significantly.
…med them, which is useless becuase we've deactivated them
…he fact that constraints might get transformed multiple times.
…nstraints than we expect (which currently we do)
…ith Vars declared as local that don't actually appear on the Disjunct, realizing that bound constraints at each level of the GDP tree matter.
@emma58 emma58 requested a review from jsiirola February 18, 2024 20:59
Comment on lines +654 to +657
original_var_info = original_var.parent_block().private_data()
if 'disaggregated_var_map' not in original_var_info:
original_var_info['disaggregated_var_map'] = ComponentMap()
disaggregated_var_map = original_var_info['disaggregated_var_map']
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should add a default= or initialize= argument to private_data() that the method can use to initialize the private dict if it is not present.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Scratch that - we implemented this as an explicit registration: #3153

@emma58 emma58 merged commit 5bfa298 into Pyomo:main Feb 21, 2024
@emma58 emma58 deleted the fix-nested-hull branch February 21, 2024 04:24
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.

2 participants