[WIP] Price-taker framework for DISPATCHES #1201

adam-a-a · 2023-06-08T19:23:30Z

Fixes

Summary/Motivation:

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.

…ow plot

MarcusHolly · 2023-06-09T20:30:33Z

@lbianchi-lbl I have two questions:

1.) Are you aware of why the Check Spelling test is failing in some PRs (like this one) but not others?

2.) We'll probably need to add some dependencies for this PR (namely scikit-learn). While this PR is still a WIP and probably won't be merged for a while, we were curious if you had any thoughts on the matter or if there are any potential complications with doing so.

lbianchi-lbl · 2023-06-09T20:52:35Z

@lbianchi-lbl I have two questions:

1.) Are you aware of why the Check Spelling test is failing in some PRs (like this one) but not others?

Thanks for bringing this to my attention. This seems to be due to a change in version of the spellchecking tool and I've created #1203 to track it. ~~It's very likely that CI checks will continue to fail until that is resolved.~~ The spellchecking issues have been resolved by #1204.

2.) We'll probably need to add some dependencies for this PR (namely scikit-learn). While this PR is still a WIP and probably won't be merged for a while, we were curious if you had any thoughts on the matter or if there are any potential complications with doing so.

Starting with #1133, optional dependencies are being managed within extras_require targets in setup.py. For the codebase currently in idaes.apps.grid_integration, the target is called grid. We can discuss more as the PR gets closer to being merged, but you can use gridx-prescient as a reference to show how to add e.g. scikit-learn.

adam-a-a · 2023-06-12T14:32:44Z

@lbianchi-lbl it seems the spell checker is identifying FOM as a typo, but it is a relevant acronym. How do we sidestep such issues, other than getting rid of the acronym?

MarcusHolly · 2023-06-12T14:34:49Z

@lbianchi-lbl it seems the spell checker is identifying FOM as a typo, but it is a relevant acronym. How do we sidestep such issues, other than getting rid of the acronym?

Is it fine to just add a few more entries to typos.toml as was done in #1204?

lbianchi-lbl · 2023-06-12T14:38:22Z

@adam-a-a @MarcusHolly that's correct. You should be able to add an entry for FOM in typos.toml, using any of the existing domain-specific acronyms and terms as a reference.

ksbeattie · 2023-06-12T16:50:05Z

@adam-a-a this doesn't look like it will make the final DISPATCHES release. Is that ok?

…-pse into adam-a-a-price-taker-model

lbianchi-lbl · 2023-06-15T18:52:38Z

@radhakrishnatg does this have already a planned completion date? Otherwise, I'll leave it off the August release board while it's still in draft.

radhakrishnatg · 2023-06-16T18:29:01Z

idaes/apps/grid_integration/multiperiod/price_taker_model.py

+        while j <= len(raw_data):
+            daily_data[day] = raw_data[i:j].reset_index(drop=True)
+            i = j
+            j = j + 24


Use horizon_length instead of hardcoding 24. If the data vector has excess elements either truncate the data or throw an exception asking the user to check the length of the data vector.

radhakrishnatg · 2023-06-16T18:29:39Z

idaes/apps/grid_integration/multiperiod/price_taker_model.py

+        return daily_data
+
+    @staticmethod
+    def get_optimal_n_clusters(daily_data, kmin=None, kmax=None, sample_weight=None):


Combine get_optimal_n_clusters and get_elbow_plot methods.

Add an argument for seed and save it as a hidden argument. Ensure that all methods which use Kmeans clustering code use the same seed value. Define seed and horizon_length as properties. You can access them getters and setters.

Update kmin and kmax to 4 and 30. Add a warning if the optimal number of clusters is close to kmax. Re-run the code with a higher kmax value in that case.

adam-a-a · 2023-06-19T17:31:42Z

@adam-a-a this doesn't look like it will make the final DISPATCHES release. Is that ok?

@ksbeattie - Actually, @radhakrishnatg coordinated with us and asked us (including @MarcusHolly and Tyler Jaffe, who should've passed the quiz and needs to be added to the repo) to deploy this. I would think we would want this in the final release, but I don't know what that release date is and have only recently gotten involved with DISPATCHES. I suppose this would be a better question for @radhakrishnatg.

adowling2 · 2023-06-19T17:42:01Z

This will get merged into the Spring 2024 IDAES release.

radhakrishnatg · 2023-06-19T17:59:49Z

@ksbeattie @adam-a-a It's okay if this PR does not make it to the final DISPATCHES release. @lbianchi-lbl We do not have a planned completion date yet. For now, let's not include this PR in the August board.

codecov · 2023-06-21T14:54:45Z

Codecov Report

Patch coverage: 36.20% and project coverage change: -0.16 ⚠️

Comparison is base (623c168) 76.83% compared to head (9b5edd4) 76.67%.

❗ Current head 9b5edd4 differs from pull request most recent head 948f40c. Consider uploading reports for the commit 948f40c to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1201      +/-   ##
==========================================
- Coverage   76.83%   76.67%   -0.16%     
==========================================
  Files         390      392       +2     
  Lines       61852    62089     +237     
  Branches    11386    11429      +43     
==========================================
+ Hits        47523    47609      +86     
- Misses      11867    12013     +146     
- Partials     2462     2467       +5

Impacted Files	Coverage Δ
.../grid_integration/multiperiod/price_taker_model.py	`33.33% <33.33%> (ø)`
...gration/multiperiod/design_and_operation_models.py	`46.34% <46.34%> (ø)`
idaes/apps/grid_integration/__init__.py	`100.00% <100.00%> (ø)`

... and 6 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

MarcusHolly · 2023-06-22T19:45:47Z

idaes/apps/grid_integration/multiperiod/price_taker_model.py

+                raw_data=raw_data[i], day_list=day_list
+            )
+
+        return daily_data, scenarios


I'm sure there's a better way to call scenarios in cluster_lmp_data without having to return it here, but if this is fine as is, we can keep it. It just makes the code a bit clunkier imo.

MarcusHolly · 2023-06-22T19:47:23Z

idaes/apps/grid_integration/multiperiod/price_taker_model.py

+
+        return daily_data, scenarios
+
+    def get_optimal_n_clusters(self, daily_data, kmin=None, kmax=None, plot=False):


Adam mentioned that we may want to reconsider having a separate function for plotting the elbow plot rather than just an argument in get_optimal_n_clusters. I don't have a strong opinion either way.

MarcusHolly · 2023-06-22T19:54:43Z

idaes/apps/grid_integration/multiperiod/price_taker_model.py

+
+        Returns:
+            lmp_data = {1: {1: 2, 2: 3, 3: 5}, 2: {1: 2, 2: 3, 3: 5}}
+            weights = {1: 45, 2: 56}


Currently the output for weights is in the format {0: {0: 45 , 1: 56, ...). I assume we want to get rid of the 0 in front so that it matches the format Radha initially put in the doc description, but I'm not sure how to go about doing that. I think it's just the header for the column similar to how the output for lmp_data is in the format: {0: {1: 2, 2: 3, 3: 5}, 1: {1: 2, 2: 3, 3: 5}}, where 0 is the header of the first column representing the first cluster.

On that last note, is it okay if lmp_data and weights start at 0 rather than 1?

MarcusHolly · 2023-06-22T19:57:27Z

idaes/apps/grid_integration/multiperiod/tests/test_price_taker.py

+
+        assert f"kmax was not set - using a default value of 30." in caplog.text
+
+    # TODO: The below test is not working because our data doesn't ever seem to arrive at an n_clusters close to kmax


I couldn't get this test working for the reason stated in the TO-DO note. I've played around with it a bit and couldn't get it working. It might be the case that we just need to generate a new dataset that is capable of arriving at an n_clusters close to kmax such that we can test this logger warning. Or maybe we should just leave this warning untested for now...

MarcusHolly · 2023-06-22T20:07:35Z

pytest.ini

 [pytest]
-addopts = --pyargs idaes
-          --durations=100
+addopts = --durations=100


Is it fine to push this change? I think the test file will fail without it since the Excel could not be imported.

ksbeattie · 2023-09-21T18:46:35Z

@adam-a-a is this still something that will be done (given that DISPATCHES is done)?

ksbeattie · 2024-06-13T18:35:54Z

@djlaky is #1358 replacing this PR? As in, should be close this one?

djlaky · 2024-06-13T18:39:31Z

@djlaky is #1358 replacing this PR? As in, should be close this one?

Yes, that is correct. This can be closed as #1358 replaces this PR.

Added basic price-taker framework

a3d688c

adam-a-a assigned adam-a-a and MarcusHolly Jun 8, 2023

adam-a-a added the DISPATCHES label Jun 8, 2023

run black

67ce6a2

adam-a-a added the WIP label Jun 8, 2023

adam-a-a and others added 3 commits June 8, 2023 16:05

add in get_optimal_n_clusters as separate method. hand off to Marcus

e716c36

Add methods for computing the optimal # of clusters and making an elb…

e3788fb

…ow plot

Merge branch 'main' into adam-a-a-price-taker-model

8ef54be

Merge branch 'main' into price-taker-model

f154700

adam-a-a added 2 commits June 12, 2023 10:53

add fom to typos.toml in github/workflows to pass spellcheck

422d0ba

fix typo

2f36bbb

MarcusHolly added 7 commits June 13, 2023 14:40

Add tests for new functions (excel import failing)

0aa1034

Update dependencies

9042e5c

Merge branch 'price-taker-model' of https://github.com/adam-a-a/idaes…

5549028

…-pse into adam-a-a-price-taker-model

Add pytest marks

2fde47f

Fix typo where TimeStep data was being used instead of BaseCaseTax

d0a6c13

Update test file

e5f6c07

Add warning for when kmax is not set

56d9369

lbianchi-lbl added the Priority:Normal Normal Priority Issue or PR label Jun 15, 2023

radhakrishnatg reviewed Jun 16, 2023

View reviewed changes

Address Radha's comments and start adding testing (WIP)

9b5edd4

MarcusHolly added 2 commits June 21, 2023 21:09

Add more tests

948f40c

Add cluster_lmp_data function

30fed63

MarcusHolly reviewed Jun 22, 2023

View reviewed changes

Minor touch-ups

29ec254

djlaky mentioned this pull request Feb 29, 2024

Price taker model for DISPATCHES, Rehashed #1358

Merged

ksbeattie closed this Jun 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Price-taker framework for DISPATCHES #1201

[WIP] Price-taker framework for DISPATCHES #1201

adam-a-a commented Jun 8, 2023

MarcusHolly commented Jun 9, 2023

lbianchi-lbl commented Jun 9, 2023 •

edited

Loading

adam-a-a commented Jun 12, 2023

MarcusHolly commented Jun 12, 2023

lbianchi-lbl commented Jun 12, 2023

ksbeattie commented Jun 12, 2023

lbianchi-lbl commented Jun 15, 2023

radhakrishnatg Jun 16, 2023 •

edited

Loading

radhakrishnatg Jun 16, 2023

radhakrishnatg Jun 16, 2023 •

edited

Loading

radhakrishnatg Jun 16, 2023 •

edited

Loading

adam-a-a commented Jun 19, 2023

adowling2 commented Jun 19, 2023

radhakrishnatg commented Jun 19, 2023

codecov bot commented Jun 21, 2023 •

edited

Loading

MarcusHolly Jun 22, 2023

MarcusHolly Jun 22, 2023

MarcusHolly Jun 22, 2023 •

edited

Loading

MarcusHolly Jun 22, 2023

MarcusHolly Jun 22, 2023

ksbeattie commented Sep 21, 2023

ksbeattie commented Jun 13, 2024

djlaky commented Jun 13, 2024


		return daily_data, scenarios

		def get_optimal_n_clusters(self, daily_data, kmin=None, kmax=None, plot=False):


		assert f"kmax was not set - using a default value of 30." in caplog.text

		# TODO: The below test is not working because our data doesn't ever seem to arrive at an n_clusters close to kmax

[WIP] Price-taker framework for DISPATCHES #1201

[WIP] Price-taker framework for DISPATCHES #1201

Conversation

adam-a-a commented Jun 8, 2023

Fixes

Summary/Motivation:

Changes proposed in this PR:

Legal Acknowledgement

MarcusHolly commented Jun 9, 2023

lbianchi-lbl commented Jun 9, 2023 • edited Loading

adam-a-a commented Jun 12, 2023

MarcusHolly commented Jun 12, 2023

lbianchi-lbl commented Jun 12, 2023

ksbeattie commented Jun 12, 2023

lbianchi-lbl commented Jun 15, 2023

radhakrishnatg Jun 16, 2023 • edited Loading

Choose a reason for hiding this comment

radhakrishnatg Jun 16, 2023

Choose a reason for hiding this comment

radhakrishnatg Jun 16, 2023 • edited Loading

Choose a reason for hiding this comment

radhakrishnatg Jun 16, 2023 • edited Loading

Choose a reason for hiding this comment

adam-a-a commented Jun 19, 2023

adowling2 commented Jun 19, 2023

radhakrishnatg commented Jun 19, 2023

codecov bot commented Jun 21, 2023 • edited Loading

Codecov Report

MarcusHolly Jun 22, 2023

Choose a reason for hiding this comment

MarcusHolly Jun 22, 2023

Choose a reason for hiding this comment

MarcusHolly Jun 22, 2023 • edited Loading

Choose a reason for hiding this comment

MarcusHolly Jun 22, 2023

Choose a reason for hiding this comment

MarcusHolly Jun 22, 2023

Choose a reason for hiding this comment

ksbeattie commented Sep 21, 2023

ksbeattie commented Jun 13, 2024

djlaky commented Jun 13, 2024

lbianchi-lbl commented Jun 9, 2023 •

edited

Loading

radhakrishnatg Jun 16, 2023 •

edited

Loading

radhakrishnatg Jun 16, 2023 •

edited

Loading

radhakrishnatg Jun 16, 2023 •

edited

Loading

codecov bot commented Jun 21, 2023 •

edited

Loading

MarcusHolly Jun 22, 2023 •

edited

Loading