Add update_model utility to utils.py for updating suffix values in Pyomo.DoE#3650
Add update_model utility to utils.py for updating suffix values in Pyomo.DoE#3650blnicho merged 35 commits intoPyomo:mainfrom
Conversation
|
@adowling2 @djlaky @blnicho |
|
This PR adds a new functionality that will be used in other PRs currently being drafted (#3575, #3639). Examples
Testing
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3650 +/- ##
=======================================
Coverage 89.18% 89.19%
=======================================
Files 890 892 +2
Lines 102778 102852 +74
=======================================
+ Hits 91663 91736 +73
- Misses 11115 11116 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mrmundt
left a comment
There was a problem hiding this comment.
I approved and then thought more about it. This doesn't have any tests. Could you please add a few, even if they are just minimal gut checks?
|
@mrmundt Absolutely, I'm working on a small example and adding checks to show it works and make sure it catches all the errors. Thank you! |
|
Main talking points for meeting 7/1:
Finished example using unknown parameters, are more needed? |
|
Action items from July 1 meeting:
|
|
@blnicho @mrmundt @djlaky @adowling2 Context: I developed the utility in doe/utils.py, and added tests to the doe/tests folder. Problem: DoE tests are failing with current example for experiment outputs and measurement error, says they are uninitialized. Proposed Solution: I made an additional example (with a parmest model) to show it works with unknown parameters, experiment outputs, and measurement_error. Question: Would it be better for me to keep everything in contrib/doe, or can I add testing and an example in parmest using a function from doe? Trying to minimize new imports needed. Thoughts? TLDR: Does it matter if I add items to doe and parmest contrib folders in same PR? Imports will overlap |
mrmundt
left a comment
There was a problem hiding this comment.
Thanks for doing all this work!
Has nothing whatsoever to do with your changes. An upstream dependency released a new version that introduced this. We're looking into it. |
All of @djlaky's comments have been resolved
Fixes # .
Summary/Motivation:
Currently, there is no way internally to update values for a suffix in a labeled model for parmest or DoE. This PR adds a new utility function to update suffix values when provided the model, the suffix, and the new values.
Changes proposed in this 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: