Skip to content

Use module enclosing generators constants#147

Merged
rouille merged 3 commits intodevelopfrom
ben/constants
Aug 25, 2020
Merged

Use module enclosing generators constants#147
rouille merged 3 commits intodevelopfrom
ben/constants

Conversation

@rouille
Copy link
Copy Markdown
Collaborator

@rouille rouille commented Aug 25, 2020

Purpose

Use the newly created powersimdata/network/usa_tamu/constants/plants.py module in the analysis and plotting modules of PostREISE. See Breakthrough-Energy/PowerSimData#267.

What is the code doing?

Use dictionaries defined in powersimdata/network/usa_tamu/constants/plants.py in place of dictionaries defined in some module (e.g. summarize.py and analyze_pg.py). Refactor modules that were using the Grid class to access generators' constants.

Where to look

Several modules were concerned in postreise/analyze/generation/ and postreise/plot/. Note that I did not refactor the code postreise/plot/multi/ for sevaeral reasons:

  • AnalyzePG objects are used and hence the code will have to be refactored once we break apart the AnalyzePG class
  • the code does not support Eastern and USA scenarios.
  • Shortfalls data and historical data are used along zones/generators constants in the constants.py module making the code very specific.
    In other words, a more extensive refactoring is necessary and beyond the scope of this PR.

Time estimate

10 min

@rouille rouille added this to the WTT90s milestone Aug 25, 2020
@BainanXia
Copy link
Copy Markdown
Collaborator

I presume the deleted notebooks are no longer used, right?

"nuclear": "Nuclear",
"geothermal": "Geothermal",
"coal": "Coal",
"dfo": "Fuel Oil",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Noticed a couple of the labels are different from the ones added in your other PR, mainly dfo and storage. Just making sure this was on purpose.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I decided to use your labeling, the one you had in the summarize module. I believe that you came up with this labeling in order to match the column name in the historical generation excel file that you load in your notebook.

I am not sure this is the best labeling though, I prefer Fuel Oil in place of DFO. We cn change that later.

One thing I don't like is to rely on excel file for the historical data (generation, demand, etc.). I would prefer to have a module that enclose dictionaries with these numbers, e.g. generation = {"Washington": {"coal": x, "ng": y, ...}, "Idaho": {"Coal": z, ...}}. We use these numbers in several places, they need to be version controlled and easily accessible.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah I think I used a copy from the notebook without realizing there were multiple versions - I also prefer Fuel Oil to DFO. Anyway sounds good, we can come back to this later.

@rouille
Copy link
Copy Markdown
Collaborator Author

rouille commented Aug 25, 2020

I presume the deleted notebooks are no longer used, right?

Yes. those were using scenarios we ran for the February 2018 review and their analysis was very specific.

Copy link
Copy Markdown
Collaborator

@BainanXia BainanXia left a comment

Choose a reason for hiding this comment

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

I think for now this is good to go.

@rouille rouille merged commit 1952527 into develop Aug 25, 2020
@rouille rouille deleted the ben/constants branch August 25, 2020 20:50
@ahurli ahurli mentioned this pull request Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants