Add Storage and Surrogate Support to the Price-taker class #1633
Conversation
| if self.config.fixed_design_data is not None: | ||
| # The design is fixed, so all the desired quantities are defined as parameters | ||
| for param_name, param_value in self.config.fixed_design_data.items(): | ||
| setattr(self, param_name, Param(initialize=param_value, mutable=True)) |
There was a problem hiding this comment.
Any reason why you chose mutable Params over fixing Vars ?
There was a problem hiding this comment.
Not quite sure which option is preferable. If we define a Var and fix it, it would allow the user to unfix it. If they do that, then the design variable would be unconstrained and it may lead to an unexpected behavior.
I feel that declaring it as a Param would prevent unfixing and it ensures that the design is always fixed. I can change it to fixing Var if that is the convention we are following everywhere.
…to pt-improvements
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1633 +/- ##
==========================================
- Coverage 77.08% 76.87% -0.21%
==========================================
Files 395 395
Lines 62759 62958 +199
Branches 10234 10277 +43
==========================================
+ Hits 48377 48402 +25
- Misses 11974 12142 +168
- Partials 2408 2414 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @declare_process_block_class("StorageModel") | ||
| class StorageModelData(ProcessBlockData): |
There was a problem hiding this comment.
Is there any reason why we don't just have this in a separate file?
| def _add_upper_bound(var_name, bound): | ||
| if isinstance(bound, (Var, Expression)): | ||
| setattr( | ||
| self, | ||
| var_name + "ub_con", | ||
| Constraint( | ||
| expr=getattr(self, var_name) <= bound, | ||
| doc=f"Constrains the maximum value of {var_name}", | ||
| ), | ||
| ) | ||
|
|
||
| else: | ||
| getattr(self, var_name).setub(bound) | ||
|
|
||
| _add_upper_bound("charge_rate", self.config.max_charge_rate) | ||
| _add_upper_bound("discharge_rate", self.config.max_discharge_rate) | ||
| _add_upper_bound("final_holdup", self.config.max_holdup) | ||
| _add_upper_bound("initial_holdup", self.config.max_holdup) |
There was a problem hiding this comment.
If I understand correctly, this _add_upper_bound function essentially provides the upper bound for a variable by adding a constraint (named m.charge_rate_ub_con in the case of charge_rate, for example) when a Var or Expression is provided.
However, when a float, int, etc., is used, this constraint would not be constructed and the upper bound of the Var would just be set directly.
While it seems like a trivial and subtle difference, it also seems less than ideal to allow variation in model structure like this (where one version includes upper bound constraints and another analogous version does not, simply based on input provided).
Is there a reason why we don't just (1) check whether instance is a Var or Expression, and if so, just do setub(pyo.value(bound)) ? The only potential issue that I can foresee with that is if an indexed Var or Expression is passed in, but haven't checked. Are there other reasons?
|
@radhakrishnatg, @adam-a-a: Moving this to the Nov release. |
|
@radhakrishnatg, @adam-a-a: Are you able now to look into this? |
|
@ksbeattie the main motivation behind this PR is related to incorporating a pricetaker model in WaterTAP. However, when pointing to this PR from WaterTAP, the current version of IDAES breaks tests on TAP. we could still bring this pr in anyhow, but a bigger issue would be getting watertap to work with latest idaes |
|
@radhakrishnatg should now be able to revisit this. |
4 participants
Summary/Motivation:
This PR adds new features to the price-taker class. This is WIP. This PR needs to be merged after merging #1579
Changes proposed in this PR:
pd.DataFrameanddict, respectively.StorageModelis found in the flowsheet instance, then linking constraints are now added automatically.Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: