-
Notifications
You must be signed in to change notification settings - Fork 28
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
Eastern hydro v3 #123
Eastern hydro v3 #123
Conversation
The ATTRIBUTION.md file looks good. That said, @kasparm, @danielolsen and I will probably need to make sure that the data used can be redistributed. |
def get_monthly_net_generation(state, eia_form_923, resource): | ||
"""Return monthly total net generation for a given resource and state from EIA | ||
form 923. | ||
def get_monthly_net_generation(state, eia_form_923, resource, hps=True): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not a big fan of mixing the resource
parameter, which is not limited to hydro with the optional hps
, which is hydro specific. On way to avoid that would be to have a resource2mover
dictionary in this module equivalent to the aall_resource
variable below that will be passed to this function in place of the resource
and hps
parameters. Then, you would then remove the HPS
value of the hydro
entry before calling the function if necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rouille Me neither. I thought about it when I was implementing this. However, the issue is the user does not necessary knows the mapping between resource and mover defined in EIA 923. Looking through the portion of the form which is used in the scope of our concern, hydro
will be the only one with one to many mapping (others are all one to one so that we could hard coded in the module), thus, I came up with the current changes with minimum modifications ( but ugly of course...). Not sure whether there are better solutions.
Should we contact the 3 ISOs (PJM, NYISO and NEISO) to make sure we can use their data? |
…rofile_by_plant_dataframe
9d9aefe
to
d254ab1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @BainanXia. Great work. It is nice to have all the methodology in the code base.
Purpose
Clear the technical debt in 2019. Merge eastern hydro profile v3 generation module into the repo.
What the code is doing
A major refactor of the demo notebook
eastern_hydro_v3_demo.ipynb
is carried out. The new notebook generates a hydro profile for eastern using the same methodology as the one in 2019 with some minor modifications, however, the resultant profiles are not exactly the same:Other changes except for the demo notebook:
--
map_grid_buses_to_county
->map_buses_to_county
--
get_profile
->get_profile_by_state
+get_profile_by_dataframe
--
get_monthly_net_generation
-> can be specified to include pumped hydro storage into hydro or not using optional boolean variablehps
develop
at 5:45 pm on 11/17/20, one conflict is resolved due to the additions in ATTRIBUTION.Testing
All tests passed. I'm not adding lots of new functions but refactor a decent amount of existing ones to make them more general. Test coverage can be improved upon requests along with this PR.
Where to look
Changes are in following directories:
Time estimate
30 min to 1 hour. A lot of data has been added. New code is mostly within notebooks, which makes it harder to review.