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

Adding private_data to _BlockData #3138

Merged
merged 6 commits into from
Feb 16, 2024
Merged

Adding private_data to _BlockData #3138

merged 6 commits into from
Feb 16, 2024

Conversation

emma58
Copy link
Contributor

@emma58 emma58 commented Feb 15, 2024

Fixes # .

Summary/Motivation:

We've maintained for awhile that we should not store state on transformation objects, and that we should instead do it on the model. That's all well and good except that we have never reserved a place for ourselves to do so. This adds a _private_data_dict attribute on _BlockData that is initialized as None, but when asked for (and given a scope (that is either the caller's module name or a parent of the caller's module name)), becomes a dictionary with keys being the name of the scope (e.g. pyomo.gdp or pyomo.gdp.bigm) and values being dictionaries to be used however the transformation/solver/anything else desires. This should simplify building and maintaining APIs for transformations to map between original and transformed components and store other state that might be needed later, and is actually private because of the restriction on keys. Even if a user used this, they would have to use keys from their scope, not ours.

Changes proposed in this PR:

  • Adds _private_data property and private_data method to _BlockData
  • Adds a test for the above.

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 emma58 requested a review from jsiirola February 15, 2024 18:45
Copy link

codecov bot commented Feb 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f81f34f) 88.34% compared to head (170acb8) 88.33%.
Report is 38 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3138      +/-   ##
==========================================
- Coverage   88.34%   88.33%   -0.01%     
==========================================
  Files         833      833              
  Lines       92567    92579      +12     
==========================================
+ Hits        81776    81779       +3     
- Misses      10791    10800       +9     
Flag Coverage Δ
linux 86.23% <100.00%> (+<0.01%) ⬆️
osx 75.69% <100.00%> (+<0.01%) ⬆️
other 86.42% <100.00%> (-0.01%) ⬇️
win 83.64% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jsiirola jsiirola merged commit 0ed5c59 into Pyomo:main Feb 16, 2024
33 checks passed
@emma58 emma58 deleted the add-mfe branch February 21, 2024 16:39
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.

3 participants