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

Price taker model for DISPATCHES, Rehashed #1358

Open
wants to merge 140 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 92 commits
Commits
Show all changes
140 commits
Select commit Hold shift + click to select a range
a3d688c
Added basic price-taker framework
radhakrishnatg May 26, 2023
67ce6a2
run black
adam-a-a Jun 8, 2023
e716c36
add in get_optimal_n_clusters as separate method. hand off to Marcus
adam-a-a Jun 8, 2023
e3788fb
Add methods for computing the optimal # of clusters and making an elb…
MarcusHolly Jun 9, 2023
8ef54be
Merge branch 'main' into adam-a-a-price-taker-model
MarcusHolly Jun 9, 2023
f154700
Merge branch 'main' into price-taker-model
lbianchi-lbl Jun 10, 2023
422d0ba
add fom to typos.toml in github/workflows to pass spellcheck
adam-a-a Jun 12, 2023
2f36bbb
fix typo
adam-a-a Jun 12, 2023
0aa1034
Add tests for new functions (excel import failing)
MarcusHolly Jun 13, 2023
9042e5c
Update dependencies
MarcusHolly Jun 13, 2023
5549028
Merge branch 'price-taker-model' of https://github.com/adam-a-a/idaes…
MarcusHolly Jun 13, 2023
2fde47f
Add pytest marks
MarcusHolly Jun 13, 2023
d0a6c13
Fix typo where TimeStep data was being used instead of BaseCaseTax
MarcusHolly Jun 15, 2023
e5f6c07
Update test file
MarcusHolly Jun 15, 2023
56d9369
Add warning for when kmax is not set
MarcusHolly Jun 15, 2023
9b5edd4
Address Radha's comments and start adding testing (WIP)
MarcusHolly Jun 21, 2023
948f40c
Add more tests
MarcusHolly Jun 22, 2023
30fed63
Add cluster_lmp_data function
MarcusHolly Jun 22, 2023
fc43a3f
linearizing su_sd cosntraints and fixing the price traker model
Aug 3, 2023
4170501
Removing files that were mistakenly saved on this branch
Aug 3, 2023
1d87aea
modified startup and shutdown constraint function
Aug 11, 2023
4710a10
add set_period to startup and shutdown func
Aug 11, 2023
be68154
developed a function to add startup/shutdown constraints
Aug 18, 2023
66db0d8
Constructed a function that adds startup and shutdown constraints
Aug 22, 2023
b39f4d9
Added deepgetattr function and had the ramp-up/down and start-up/down…
Aug 30, 2023
799ed0b
linearized ramping constraints
Sep 14, 2023
7625d07
Added auxiliary variables
Sep 14, 2023
0c2c461
added a third mccor constraint for each aux var and created a capacit…
Sep 18, 2023
16710a6
changed startup_shutdown constraints to have the shutdown binary var
Sep 19, 2023
4518240
changed rule for min_start_up constraint
Sep 19, 2023
35ddead
fixed startup_shutdown function constraints, changed default values f…
Sep 20, 2023
fd15af9
added new method for adding aux variables
Sep 21, 2023
b84ea43
removed aux variable delcaration and added a new config option to opt…
Sep 21, 2023
15ea641
added comments
Sep 21, 2023
53e427e
small addition
Sep 21, 2023
14ee544
added design_blk
Sep 21, 2023
4e10801
added deepgetattr and constraint for the add capacity aux var func
Sep 21, 2023
d68d8fd
finished the new method and ramping function
Sep 22, 2023
6cd966e
Allowed _add_capacity_aux_vars to be cosntructed when the build func…
Oct 4, 2023
f7e3245
added a for loop that calls on the _add_capacity_aux_vars function t …
Oct 4, 2023
b223cf6
Pulling Tyler\'s brnach
Oct 31, 2023
56f8099
Updating PT model for SOFC case study
Nov 1, 2023
48d4d98
Updated price_taker_model.py doc
djlaky Feb 15, 2024
fde3f31
Updated init of PriceTakerModel
djlaky Feb 15, 2024
5bd0d5b
Added missing documentation
djlaky Feb 15, 2024
0df8b45
Corrected constraint expressions
djlaky Feb 15, 2024
fd595b4
Added tests for input checking
djlaky Feb 15, 2024
a54c3ac
Updated checks for get_optimal_clusters
djlaky Feb 15, 2024
70cbaba
Fixed typos in test_price_taker.py
djlaky Feb 15, 2024
85b7bd0
Added input checks on add_ramping_constraints
djlaky Feb 15, 2024
9851b88
Fixed broken test
djlaky Feb 15, 2024
6ede14e
Added automated LMP population
djlaky Feb 16, 2024
d31b272
Fixed getitem issues with Sets
djlaky Feb 16, 2024
769ba7b
Removed redundant set definition
djlaky Feb 16, 2024
0bba4d2
Added n_clusters check in append_lmp_data
djlaky Feb 16, 2024
b06d467
Reorganizing unit logging messages tests
djlaky Feb 20, 2024
0cf82bf
Moved deepgetattr to import from design_and_operation_models.py
djlaky Feb 21, 2024
6af8c9e
Added checks and tests
djlaky Feb 21, 2024
1fc5f53
Updated data processing workflow
djlaky Feb 22, 2024
e991392
Small update to append_lmp_data
djlaky Feb 22, 2024
f2725cc
Updated get_optimal_n_clusters
djlaky Feb 27, 2024
522023b
Updated cash flow construction
djlaky Feb 27, 2024
22dbe12
Added tests for cashflows
djlaky Feb 27, 2024
6dbb183
Fixed typos and ran black
djlaky Feb 29, 2024
7a86d03
Run black
djlaky Feb 29, 2024
7a24fba
Fixed some broken tests
djlaky Feb 29, 2024
b1629eb
Removed a test that is failing
djlaky Feb 29, 2024
d193e65
Reverted pytests.ini and corrected tests
djlaky Mar 4, 2024
fb543c7
Updated plotting test
djlaky Mar 4, 2024
460af99
Ran Black again
djlaky Mar 4, 2024
c90bbe1
Merge branch 'IDAES:main' into price-taker-model
djlaky Mar 4, 2024
643707f
Updating tests
djlaky Mar 5, 2024
8aa02c6
Add capacity limits function
djlaky Mar 25, 2024
323e4a9
Added final docstring
djlaky Mar 25, 2024
04d07ae
Ran black
djlaky Mar 25, 2024
4ae5779
Bugfix capacity constraints
djlaky Mar 25, 2024
893743d
Removed multiyear LMP support
djlaky Mar 27, 2024
82f8bdc
Merge branch 'IDAES:main' into price-taker-model
djlaky Mar 27, 2024
ebdc1b2
Delete idaes/apps/grid_integration/multiperiod/ERCOT_WEST_2022_shutdo…
djlaky Mar 28, 2024
aad41fb
Update docstring for n_clusters
djlaky Apr 30, 2024
e2dac86
Fixed typos.
djlaky Apr 30, 2024
5ad727e
Removed design_blk as an input for Operation_Model
djlaky Apr 30, 2024
2ae4635
Fixed typo in error message in tests
djlaky Apr 30, 2024
8762f7b
Replace deepgetattr with find_component()
djlaky Apr 30, 2024
7110dea
Changed dependence of tests on .xlsx to .csv
djlaky Apr 30, 2024
8550569
Merge branch 'IDAES:main' into price-taker-model
djlaky Apr 30, 2024
69c3c9b
Changed deepgetattr functionality to find_component
djlaky Apr 30, 2024
177618c
Merge branch 'price-taker-model' of https://github.com/dlakes94/idaes…
djlaky Apr 30, 2024
ce4b0fb
Updated test to remove .xlsx dependencies
djlaky Apr 30, 2024
343d191
Update on build_hourly_cashflows docstring
djlaky Apr 30, 2024
cec302e
Update for Sphinx again
djlaky Apr 30, 2024
652d1cc
Ran black
djlaky Apr 30, 2024
d8ade5d
Updated SkeletonUnitModelData to ProcessBlockData
djlaky May 8, 2024
cd3f3d3
Made sklearn and kneed optional, added log msgs
djlaky May 8, 2024
ce6b3db
Updated tests for price taker class
djlaky May 8, 2024
f4cfbdb
Fixed optional import tests
djlaky May 8, 2024
cbd7aaa
Ran black
djlaky May 8, 2024
f10ea59
Bugfix kmeans for LMP
djlaky May 16, 2024
46a3331
merged idaes main
May 29, 2024
7bdd94d
Revert "merged idaes main"
djlaky Jun 10, 2024
1c85981
Add initial documentation for pricetaker
MarcusHolly Jul 12, 2024
6956d73
Add autoclass to documentation
MarcusHolly Jul 12, 2024
8b4b479
Address merge conflict
MarcusHolly Jul 12, 2024
646b88b
Fix typos in documentation
MarcusHolly Jul 12, 2024
a1f1eb2
Reformat how math equations are handled
MarcusHolly Jul 12, 2024
8101448
add subtle additions to price-taker model used in workhop
adam-a-a Jul 15, 2024
d8de0dd
Merge branch 'main' into price-taker-model
adam-a-a Jul 23, 2024
fc7369e
Test for seed instance and seed ValueError
MarcusHolly Sep 5, 2024
75a56f4
Check for expected string outputs rather than using f-strings
MarcusHolly Sep 5, 2024
df687af
Separate tests that check for multiple error messages
MarcusHolly Sep 5, 2024
47b66c5
Separate a majority of the code outside of the pytest.raises
MarcusHolly Sep 6, 2024
8ac5987
Update imports
MarcusHolly Sep 9, 2024
d10b9a7
Merge branch 'main' into price-taker-model
MarcusHolly Sep 9, 2024
1e7842a
Add version check for sklearn
MarcusHolly Sep 9, 2024
8b1393a
Add separate function & test for generating elbow plots
MarcusHolly Sep 9, 2024
6481545
Refines version testing and # of optimal clusters testing
MarcusHolly Sep 11, 2024
b9f3ef4
Remove unused imports
MarcusHolly Sep 11, 2024
bd2db6f
Merge branch 'price-taker-model' of https://github.com/djlaky/idaes-p…
MarcusHolly Sep 11, 2024
9cf4a3a
Merge branch 'main' into djlaky-price-taker-model
MarcusHolly Oct 15, 2024
112761f
Correct acronym in pricetaker documentation
MarcusHolly Oct 15, 2024
fb0a05b
Remove commented code in design_and_operation_models
MarcusHolly Oct 15, 2024
5d960a4
Add dependencies for scikit-learn and kneed
MarcusHolly Oct 15, 2024
d7a0002
Update imports and seed.setter method
MarcusHolly Oct 15, 2024
727be78
Resolve test failures
MarcusHolly Oct 16, 2024
aebe5b8
Update pricetaker to not create new dependencies
MarcusHolly Oct 17, 2024
411bb60
Update pricetaker testing based on previous commit
MarcusHolly Oct 17, 2024
bd3244c
Refine docstring for compute_sse method
MarcusHolly Oct 17, 2024
b8149b5
Make compute_sse a prviate method & move outside of class
MarcusHolly Oct 17, 2024
043ac34
Update compute_sse docstring
MarcusHolly Oct 17, 2024
2cd22e4
Added tests for design and operation model classes
radhakrishnatg Oct 23, 2024
e9ff144
repair failing check by updating config option name; also update exce…
adam-a-a Oct 30, 2024
422ec6e
black
adam-a-a Oct 30, 2024
042a1d9
add more exception handling
adam-a-a Oct 30, 2024
0fd8edb
Update docs/reference_guides/apps/grid_integration/multiperiod/Price_…
MarcusHolly Oct 30, 2024
892e555
Merge branch 'main' into price-taker-model
adam-a-a Oct 30, 2024
a2c97c4
blk
adam-a-a Oct 30, 2024
20db58f
try to fix docstrings to mitigate sphinx error
adam-a-a Oct 30, 2024
a4620de
Remove unnecessary f-strings
MarcusHolly Nov 5, 2024
a1ae6b7
Remove/re-add commented code in price taker testing
MarcusHolly Nov 5, 2024
bef17fb
Add logger messages for when model_func is not defined
MarcusHolly Nov 8, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/typos.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ equil = "equil"
astroid = "astroid"
# delt == delta
delt = "delt"
# FOM for fixed operating and maintenance for DISPATCHES
FOM = "FOM"
fom = "fom"
# TEMPORARY - Remove after example update
upadate = "upadate"

Expand Down
5 changes: 5 additions & 0 deletions idaes/apps/grid_integration/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,8 @@
from .coordinator import DoubleLoopCoordinator
from .forecaster import PlaceHolderForecaster
from .multiperiod.multiperiod import MultiPeriodModel
from .multiperiod.price_taker_model import PriceTakerModel
from .multiperiod.design_and_operation_models import (
DesignModel,
OperationModel,
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
#################################################################################
# The Institute for the Design of Advanced Energy Systems Integrated Platform
# Framework (IDAES IP) was produced under the DOE Institute for the
# Design of Advanced Energy Systems (IDAES).
#
# Copyright (c) 2018-2023 by the software owners: The Regents of the
# University of California, through Lawrence Berkeley National Laboratory,
# National Technology & Engineering Solutions of Sandia, LLC, Carnegie Mellon
# University, West Virginia University Research Corporation, et al.
# All rights reserved. Please see the files COPYRIGHT.md and LICENSE.md
# for full copyright and license information.
#################################################################################

from pyomo.environ import (
Var,
Param,
Binary,
Expression,
NonNegativeReals,
Constraint,
)
from functools import reduce
MarcusHolly marked this conversation as resolved.
Show resolved Hide resolved
from idaes.core.base.process_base import declare_process_block_class
from idaes.models.unit_models import SkeletonUnitModelData
from pyomo.common.config import ConfigValue, In


@declare_process_block_class("DesignModel")
class DesignModelData(SkeletonUnitModelData):
Copy link
Member

Choose a reason for hiding this comment

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

Out of interest, why are you inheriting SkeletonUnitModelData here and not UnitModelData (or another base class)?

Copy link
Author

Choose a reason for hiding this comment

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

I believe this was the first thing that came to mind when the original development was happening. @radhakrishnatg can probably comment more on this.

Copy link
Contributor

Choose a reason for hiding this comment

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

@andrewlee94 From what I remember, UnitModelData class has several additional methods that I think are not needed. I just need a simple Pyomo block. If UnitModelData is a better alternative, then @djlaky, please make the change.

Copy link
Member

Choose a reason for hiding this comment

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

It might be OK, I was mostly curious (and thought that SkeletonUnitModel might have inherited from UnitModelBlock anyway, but I just checked and it does not). However, do you use the methods defined on SkeletonUnitModel at all (add_ports, fix_initialization_states, initialize (deprecated)), or would ProcessBlock be just as good (the parent of SkeletonUnitModel)?

"""
Class for containing design model ...
Copy link
Member

Choose a reason for hiding this comment

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

I still do not know enough about what this class actually does to approve this. The docs do not explain what this is or how to use it, and I am not sure what this code actually adds (i.e. is it even necessary?).

For one, this needs a descriptive doc string to explain what it does.

Copy link
Contributor

Choose a reason for hiding this comment

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

@radhakrishnatg would be the best person to respond to this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Based on discussions, @radhakrishnatg explained the need for these classes, particularly with regard to cost calculations later on. @andrewlee94 are there other particulars that you want addressed, aside from further documentation?

Copy link
Member

Choose a reason for hiding this comment

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

@radhakrishnatg and I have discussed this. The first big thing is some basic documentation so that users and maintainers can understand what they are looking at (this can just be better doc strings). I also pointed out that the same basic interface could be done using a more standard approach of having the users write derived classes instead of the current approach of providing a method as an argument (it adds 2 or 3 lines of boilerplate code, but aligns this tool with all the rest of IDAES).

I would also like to make sure there is a maintenance plan for this and an assigned maintainer.

Copy link
Contributor

Choose a reason for hiding this comment

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

@andrewlee94 Do you mean that we would have Design and Operation model classes as is, with the model build config options (and associated code) removed, then build model specific classes? For example, NGCC design model would inherit from DesignModel class, and it would merely add the required code under the build method to construct the model; similarly, NGCC operation model would inherit from OperationModel and add required code under build method. Is that along the lines of what you were thinking?

In the interim, we would be maintaining this code for flex desal work, but I imagine that there are other activities that would make use of this code as well, now and in the future. I guess the assigned maintainer(s) would be up for discussion. Arguably, it could be @radhakrishnatg , @MarcusHolly , and myself.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that was my suggestion as it is almost identical to writing the method that is passed in as an argument.

My main reason is because this is how the other IDAES models are written and so aligns this with the standard. Otherwise we have this one section of the code that does things differently which will end up confusing someone at some point.

For the transition, you could support both - someone writing the build method just does not provide the model build config. However, you would want to add a deprecation to the model_build pathway and not document it (which is easy as there is no documentation yet).

Copy link
Contributor

Choose a reason for hiding this comment

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

I am pasting my response here for better visibility:

@andrewlee94 I was just thinking about this, and on the one hand, using derived classes would be more in line with how the rest of IDAES does things. On the other hand, using this model_func approach gives the potential of applying either approach: I can formulate quick and dirty models on the fly by using the config option to pass in a build function, or I can go and create a child class. So in a sense, allowing for the current approach would give more flexibility, while eliminating it would be more restrictive on the user (which might be the way to go in most cases, but not necessarily here).

In any case, I think that any refactor of this formulation should be part of a subsequent PR. I think it is more important for this overall capability to get merged in first, since there has already been published work on this Pricetaker approach (for one).

Copy link
Contributor

Choose a reason for hiding this comment

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

@andrewlee94 So I would propose the following:

  • Finalize addition of tests to ensure existing formulation functions as expected
  • Finalize documentation of current formulation, be it through a technical ref or doc strings
  • Merge PR
  • Follow up with a subsequent PR that adds, say, derived design and operation classes for a given example, update documentation to reflect this, and describe both alternatives---the conventional way as well as the on-the-fly approach
  • Later decide on deprecation pathway or whether we want to maintain both approaches

Copy link
Member

Choose a reason for hiding this comment

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

@adam-a-a My counter to the argument against derived classes is that from a code perspective there are only two additional lines of boilerplate code, and thus derived classes are effectively no harder that the model_build approach. Consider:

def my_func():
    # My code

is equivalent to:

@declare_process_block_class(MyPriceTaker)
class MyPriceTakerData(PriceTakerBase):
    def build():
        # My code

The one difference is how arguments to the method would be handled, but even that is fairly easy. For the derived class approach, the code just needs to refer to self.config.model_build_arguments (which already exists) instead of expecting the mas direct arguments.

I do agree that this could be done in a later PR however. The main reason this came up was that it is not particularly well documented how this was intended to be used so I wanted clarification on useage for this PR.

"""

CONFIG = SkeletonUnitModelData.CONFIG()
CONFIG.declare(
"model_func",
ConfigValue(
doc="Function that builds the design model",
),
)
CONFIG.declare(
"model_args",
ConfigValue(
default={},
doc="Dictionary containing arguments needed for model_func",
),
)

# noinspection PyAttributeOutsideInit
MarcusHolly marked this conversation as resolved.
Show resolved Hide resolved
def build(self):
super().build()

self.config.model_func(self, **self.config.model_args)
adam-a-a marked this conversation as resolved.
Show resolved Hide resolved


@declare_process_block_class("OperationModel")
class OperationModelData(SkeletonUnitModelData):
"""
Class for containing design model ...
"""

CONFIG = SkeletonUnitModelData.CONFIG()
CONFIG.declare(
"model_func",
ConfigValue(
doc="Function that builds the design model",
),
)
CONFIG.declare(
"model_args",
ConfigValue(
default={},
doc="Dictionary containing arguments needed for model_func",
),
)

CONFIG.declare(
"declare_op_vars",
ConfigValue(
default=True,
domain=In([True, False]),
doc="Should op_mode, startup, shutdown vars be defined?",
),
)
CONFIG.declare(
"append_lmp_data",
ConfigValue(
default=True,
domain=In([True, False]),
doc="Should LMP data automatically be appended to the model?",
),
)

# noinspection PyAttributeOutsideInit
def build(self):
Copy link
Member

Choose a reason for hiding this comment

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

Similarly, the usage of this class will need to be clearly documented, especially what model_func is expected to do. I am again wondering if it would be better to get the user to inherit from this and just write model_func as their build method.

Copy link
Contributor

Choose a reason for hiding this comment

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

@andrewlee94 I was just thinking about this, and on the one hand, using derived classes would be more in line with how the rest of IDAES does things. On the other hand, using this model_func approach gives the potential of applying either approach: I can formulate quick and dirty models on the fly by using the config option to pass in a build function, or I can go and create a child class. So in a sense, allowing for the current approach would give more flexibility, while eliminating it would be more restrictive on the user (which might be the way to go in most cases, but not necessarily here).

In any case, I think that any refactor of this formulation should be part of a subsequent PR. I think it is more important for this overall capability to get merged in first, since there has already been published work on this Pricetaker approach (for one).

Copy link
Contributor

Choose a reason for hiding this comment

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

In any case, I think that any refactor of this formulation should be part of a subsequent PR. I think it is more important for this overall capability to get merged in first, since there has already been published work on this Pricetaker approach (for one).

This approach is backwards. If we know a refactor is coming, it's better to do it before the PR is merged. Otherwise we need to deprecate the old class, leave it in a few cycles, and create a similarly-named new class in order to maintain backwards compatibility.

Copy link
Contributor

Choose a reason for hiding this comment

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

In fact, we don't know a refactor is coming and it is something that is really up for discussion at this point--which is why I said "if any refactor." This PriceTaker class has been delayed for quite some time now, and I don't think we should wait until the discussion settles.

super().build()

# Build the model
if self.config.declare_op_vars:
self.op_mode = Var(
within=Binary,
doc="Binary var: 1 if the unit is operating, 0 otherwise",
)

self.startup = Var(
within=Binary,
doc="Binary var: 1 if the startup is initiated, 0 otherwise",
)

self.shutdown = Var(
within=Binary,
doc="Binary var: 1 if the shutdown is initiated, 0 otherwise",
)

if self.config.append_lmp_data:
self.LMP = Param(
initialize=1,
mutable=True,
doc="Parameter: Will be updated to LMP value at given time",
)

self.config.model_func(self, **self.config.model_args)
radhakrishnatg marked this conversation as resolved.
Show resolved Hide resolved
Loading
Loading