Renewable + PEM Parameterized Bidder #1407

Xinhe-Chen · 2024-05-09T17:01:53Z

Fixes

Add the renewable+PEM parameterized bidder (bids the power at a constant price).

Summary/Motivation:

Add a PerfectForecaster to predict the day-ahead and real-time wind power and electricity prices (if needed).
Add a PEMParameterizedd bidder to bid the renewable power at a constant prices.

Changes proposed in this PR:

Legal Acknowledgement

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

I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
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.

…bidder

…into param_bidder

andrewlee94

A number of requests regarding improvements to testing.

andrewlee94 · 2024-05-09T17:07:58Z

idaes/apps/grid_integration/tests/test_perfectforecaster.py

+    pyo_unittest.assertStructuredAlmostEqual(
+        first=result_forecast.tolist(), second=expected_forecast
+    )
+    return


Tests should not return anything - this causes a lot of warnings to be emitted in the test output which makes it hard to find real issues. There are 4 more instances of this below as well.

andrewlee94 · 2024-05-09T17:09:34Z

idaes/apps/grid_integration/tests/test_perfectforecaster.py

+@pytest.mark.unit
+@pytest.mark.parametrize("value", [np.array([1,2,3,4,5]), [1,2,3,4,5]])
+def test_create_perfectforecaster_with_ndarray_and_list(value):
+    with pytest.raises(ValueError):


When checking for errors, you should also include the expected text message to ensure you got the right error (and not another of the same type).

andrewlee94 · 2024-05-09T17:11:15Z

idaes/apps/grid_integration/tests/test_perfectforecaster.py

+    return PerfectForecaster(wind_df)
+
+@pytest.mark.unit
+def test_create_perfectforecaster(wind_df):


These are more like component tests than unit tests. Unit tests should generally be focused on individual methods/functions in the code to make sure all the elements work correctly in isolation, followed by a few test cases like these to make sure the end-to-end workflow is correct.

@Xinhe-Chen, I think you already did this. Is so, just reply here with a short sentence confirming/summarizing what you did.

I have addressed this.

andrewlee94 · 2024-05-09T17:12:38Z

idaes/apps/grid_integration/tests/test_PEM_Parameterized_bidder.py

+    TestingForecaster,
+    testing_model_data,
+)
+from pyomo.common import unittest as pyo_unittest


Group imports together, starting with native Python packages, then third-party packages, then Pyomo and finally IDAES.

andrewlee94 · 2024-05-09T17:13:12Z

idaes/apps/grid_integration/tests/test_PEM_Parameterized_bidder.py

+
+day_ahead_horizon = 24
+real_time_horizon = 4
+solver = pyo.SolverFactory("cbc")


You should probably add a check to ensure cbc is available in the test environment and to skip these tests if it is not.

andrewlee94 · 2024-05-09T17:13:50Z

idaes/apps/grid_integration/tests/test_PEM_Parameterized_bidder.py

+    return bidder_object
+
+
+# @pytest.mark.unit


Commented code.

Also, there is not a lot of testing for this class here - there are a number of methods that should be unit tested.

andrewlee94 · 2024-05-09T17:14:42Z

Also, we will need documentation for the new classes and how to use them.

Xinhe-Chen · 2024-05-09T17:28:28Z

Also, we will need documentation for the new classes and how to use them.

Thank you for your comments! I am still working on improving the tests. The documentation will be added in this PR #1090.

Xinhe-Chen · 2024-05-16T18:30:11Z

@radhakrishnatg Hi Radhakrishna, would you please review this PR? Thank you!

Xinhe-Chen · 2024-05-20T16:38:53Z

@adowling2 this is ready for review.

Xinhe-Chen · 2024-06-26T18:50:22Z

@adowling2 This is ready for review.

Xinhe-Chen · 2024-06-27T18:12:03Z

@ksbeattie Hi Keith, we hope to have this PR reviewed and merged next week.

adowling2

Please see the comments below. These are all asking for minor changes, often just more descriptive comments.

adowling2 · 2024-06-28T17:32:45Z

docs/reference_guides/apps/grid_integration/Bidder.rst

@@ -13,6 +13,155 @@ uncertain price scenario has a corresponding power output. As shown in the figur
 each of these uncertain price and power output pairs formulates a segment in the
 bidding curves.

+Here we present a stochastic bidding model for a renewable integrated energy system (Wind generator + PEM).


wind should be lowercase, make sure you define PEM on first use

This is addressed.

adowling2 · 2024-06-28T17:33:03Z

docs/reference_guides/apps/grid_integration/Bidder.rst

+in order to avoid underbidding. Equation (2) states that the RT offering power is the same as the 
+IES power output to the grid. In the bidding mode, the market rules require the offering power is 
+non-decreasing (convex) with its marginal cost in an energy bid. This rule is represented by equation (3).
+Equation (4) to equation (9) are process model constraints. Equation (10) calculates the operation costs for IES


Equations (4) to (9)

adowling2 · 2024-06-28T17:35:19Z

idaes/apps/grid_integration/bidder.py

+        This function records the bids and the details in the underlying bidding model.
+
+        Arguments:
+            bids: the obtained bids for this date.


What is the specific data structure for a bid? Is this a list? Please provide a little more information in the comments

Type hinting would be a good idea. E.g. the method signature could be:

def record_bids(self, bids: list, model, date, hour, market):

I added variable types in code comments.

adowling2 · 2024-06-28T17:35:33Z

idaes/apps/grid_integration/bidder.py

+
+            model: bidding model
+
+            date: the date we bid into


Is this a string or something else?

Typing hinting will help again. Also, remove the line breaks between each argument (otherwise the docs formatter might get confused).

adowling2 · 2024-06-28T17:35:43Z

idaes/apps/grid_integration/bidder.py

+
+            date: the date we bid into
+
+            hour: the hour we bid into


Is this an integer or something else?

adowling2 · 2024-06-28T17:40:44Z

idaes/apps/grid_integration/bidder.py

+    ):
+        """
+        Arguments:
+            renewable_mw: maximum renewable energy system capacity


Is this a float? If so, add (float) or , float to the end of the doc string.

adowling2 · 2024-06-28T17:40:52Z

idaes/apps/grid_integration/bidder.py

+        Arguments:
+            renewable_mw: maximum renewable energy system capacity
+
+            pem_marginal_cost: the cost/MW above which all available wind energy will be sold to grid;


Is this a float?

adowling2 · 2024-06-28T17:41:00Z

idaes/apps/grid_integration/bidder.py

+            pem_marginal_cost: the cost/MW above which all available wind energy will be sold to grid;
+                below which, make hydrogen and sell remainder of wind to grid
+
+            pem_mw: maximum PEM capacity limits how much energy is bid at the `pem_marginal_cost`


Is this a float?

adowling2 · 2024-06-28T17:42:49Z

idaes/apps/grid_integration/forecaster.py

+         - {bus}-DALMP
+         - {bus}-RTLMP
+         - {bus}-DACF and {bus}-RTCF for renewable plants
+        """


Perhaps add a sentence explaining the perfect forecast has no error and "sees into the future".

@Xinhe-Chen, did you do this? If so, please reply here.

adowling2 · 2024-06-28T17:43:47Z

idaes/apps/grid_integration/tests/test_PEM_Parameterized_bidder.py

+                (expected_DA_cf[t] * pmax, pem_marginal_cost),
+            ]
+        expect_cost_curve = convert_marginal_costs_to_actual_costs(expect_bids_curve)
+        expected_bids[t] = {


Was this comment addressed? If so, reply here.

adowling2

@Xinhe-Chen, please reply to the open comments to confirm these were addressed.

adowling2 · 2024-07-01T17:13:18Z

idaes/apps/grid_integration/bidder.py

+            None
+
+        """
+        df_list = []


@Xinhe-Chen, did you address this comment? If so, please reply here.

adowling2 · 2024-07-01T17:13:45Z

idaes/apps/grid_integration/forecaster.py

+         - {bus}-DALMP
+         - {bus}-RTLMP
+         - {bus}-DACF and {bus}-RTCF for renewable plants
+        """


@Xinhe-Chen, did you do this? If so, please reply here.

adowling2 · 2024-07-01T17:14:00Z

idaes/apps/grid_integration/tests/test_PEM_Parameterized_bidder.py

+                (expected_DA_cf[t] * pmax, pem_marginal_cost),
+            ]
+        expect_cost_curve = convert_marginal_costs_to_actual_costs(expect_bids_curve)
+        expected_bids[t] = {


@Xinhe-Chen Please reply here if you addressed this.

adowling2 · 2024-07-01T17:14:41Z

idaes/apps/grid_integration/tests/test_perfectforecaster.py

+    return PerfectForecaster(wind_df)
+
+@pytest.mark.unit
+def test_create_perfectforecaster(wind_df):


@Xinhe-Chen, I think you already did this. Is so, just reply here with a short sentence confirming/summarizing what you did.

Xinhe-Chen · 2024-07-01T17:27:20Z

@Xinhe-Chen, please reply to the open comments to confirm these were addressed.

Hi Prof. Dowling, I addressed all comments from you last Friday.

I am going to to a final check later today to ensure no other issues are remaining later today.

adowling2 · 2024-07-01T17:31:07Z

Excellent. Tag me here when it is ready to review.

Xinhe-Chen · 2024-07-01T20:22:30Z

Excellent. Tag me here when it is ready to review.

@adowling2 This is ready for review.

adowling2

Ready to merge.

andrewlee94

@Xinhe-Chen I have one final request before we merge this; someone should be added to the top level CODEOWNERS file as the maintainer for this code (i.e. the person Keith or Ludovico will contact if tests start failing). We require all code in apps to have an assigned owner for maintenance purposes.

andrewlee94 · 2024-07-01T20:35:34Z

idaes/apps/grid_integration/bidder.py

+            None
+        """
+
+        print("")


Using a logger is generally preferred to doing print statements as this gives the end-user more flexibility in how (and what) things are displayed.

Also, for a new line use print("\n"}.

https://github.com/IDAES/idaes-pse/blob/main/.github/CODEOWNERS

I saw @adowling2 is on the CODEOWNERS roster for the grid_integration codes. I am a PhD student and will graduate in 2 years but I am happy to be responsible for maintaining these codes until I graduate.

Using a logger is generally preferred to doing print statements as this gives the end-user more flexibility in how (and what) things are displayed.

Also, for a new line use print("\n"}.

Done.

I see that you changed the new line, but what about using a logger?

I am not very familiar with the logger, so I need to look into that.

Hi Andrew, I used logger to replace print and it works. Thank you for letting me know this.

Xinhe-Chen · 2024-08-01T18:56:13Z

@andrewlee94 Hi Andrew, I think I have already addressed your comments. Shall we get this PR merged?

andrewlee94

Sorry for the delay, I was at a conference or out on leave the past three weeks. A couple of minor requests for changes to how you are using the logger.

idaes/apps/grid_integration/bidder.py

Xinhe-Chen · 2024-08-08T17:24:27Z

Sorry for the delay, I was at a conference or out on leave the past three weeks. A couple of minor requests for changes to how you are using the logger.

Thank you! I have addressed your new comments.

Xinhe-Chen and others added 8 commits May 2, 2024 15:53

improve the PEM parameterized bidder

fd8a16d

improve pem bidder and add some tests

ac96407

update the logic and fix bugs

a9db27d

Merge branch 'main' into param_bidder

681e2b5

change the test file names

7477b28

Merge branch 'main' of https://github.com/IDAES/idaes-pse into param_…

3d59d6b

…bidder

Merge branch 'param_bidder' of https://github.com/Xinhe-Chen/idaes-pse …

880fe69

…into param_bidder

use black

f30b2b5

andrewlee94 requested changes May 9, 2024

View reviewed changes

update tests

351bde0

ksbeattie added the Priority:Normal Normal Priority Issue or PR label May 9, 2024

Xinhe-Chen and others added 5 commits May 14, 2024 13:12

Merge branch 'main' into param_bidder

9e779ae

update tests

f6ac3e0

update black and test

5d9a205

add more tests

0011b8f

move the docs to this branch

aed28b8

Xinhe-Chen mentioned this pull request May 14, 2024

Add Wind+PEM tracking and stochastic bidding documentation #1090

Closed

Xinhe-Chen added 3 commits May 16, 2024 11:54

update docs

fba6171

update the docs and code comments

92e4b5e

updates

a0c11be

adam-a-a requested a review from radhakrishnatg May 16, 2024 17:01

fix doc typos

4639ecc

ksbeattie assigned Xinhe-Chen May 23, 2024

adowling2 marked this pull request as ready for review May 28, 2024 17:53

adowling2 requested review from jsiirola and adowling2 as code owners May 28, 2024 17:53

Merge branch 'main' into param_bidder

365c72d

adowling2 requested changes Jun 28, 2024

View reviewed changes

Xinhe-Chen added 4 commits June 28, 2024 15:16

add/change comments as Alex suggests

25048f5

update docs

0e13af6

use black to format codes

4db3cac

fix typos

a157a58

adowling2 self-requested a review July 1, 2024 17:11

adowling2 reviewed Jul 1, 2024

View reviewed changes

fix typos

a4e9b49

adowling2 approved these changes Jul 1, 2024

View reviewed changes

andrewlee94 reviewed Jul 1, 2024

View reviewed changes

Xinhe-Chen and others added 4 commits July 1, 2024 17:06

minor change

31f6e46

use logger

cbb7244

Merge branch 'main' into param_bidder

e347b24

Merge branch 'main' into param_bidder

1e2b0a0

Merge branch 'main' into param_bidder

58b2827

andrewlee94 requested changes Aug 6, 2024

View reviewed changes

idaes/apps/grid_integration/bidder.py Outdated Show resolved Hide resolved

idaes/apps/grid_integration/bidder.py Outdated Show resolved Hide resolved

use idaeslogger

eaf075f

andrewlee94 approved these changes Aug 8, 2024

View reviewed changes

ksbeattie enabled auto-merge (squash) August 8, 2024 18:30

update for site-packages installation

c22317a

lbianchi-lbl disabled auto-merge August 12, 2024 17:31

lbianchi-lbl merged commit 04fa887 into IDAES:main Aug 12, 2024
43 checks passed

Renewable + PEM Parameterized Bidder #1407

Renewable + PEM Parameterized Bidder #1407

Conversation

Xinhe-Chen commented May 9, 2024

Fixes

Summary/Motivation:

Changes proposed in this PR:

Legal Acknowledgement

andrewlee94 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewlee94 commented May 9, 2024

Xinhe-Chen commented May 9, 2024

Xinhe-Chen commented May 16, 2024

Xinhe-Chen commented May 20, 2024

Xinhe-Chen commented Jun 26, 2024

Xinhe-Chen commented Jun 27, 2024

adowling2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adowling2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Xinhe-Chen commented Jul 1, 2024

adowling2 commented Jul 1, 2024

Xinhe-Chen commented Jul 1, 2024

adowling2 left a comment

Choose a reason for hiding this comment

andrewlee94 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Xinhe-Chen commented Aug 1, 2024

andrewlee94 left a comment

Choose a reason for hiding this comment

Xinhe-Chen commented Aug 8, 2024