feat: analyze curtailment #57
Conversation
postreise/analyze/curtailment.py
Outdated
| """Ensure that curtailment is a dict of dataframes, and that each key is | ||
| represented in at least one generator in grid. | ||
| :param dict curtailment: keys are resources, values are pandas.DataFrame. | ||
| :param powersimdata.input.grid grid: Grid instance. |
There was a problem hiding this comment.
Pycharm is saying you need to use :param powersimdata.input.grid.Grid here and in the rest of the docstrings
There was a problem hiding this comment.
Buffalo buffalo Buffalo buffalo buffalo buffalo Buffalo buffalo
There was a problem hiding this comment.
I prefer the :param powersimdata.scenario.scenario.Scenario scenario:
| _resource_func = {'solar': 'get_solar', 'wind': 'get_wind'} | ||
|
|
||
|
|
||
| def _check_scenario(scenario): |
There was a problem hiding this comment.
I personally write custom checks on inputs when having the wrong input would cause a really confusing error or possibly a silent bug, for example when an array needs to be a certain length. So I'm not sure you need this -- I think calling if not isinstance(scenario.state, Analyze) should cause a pretty clear error on its own.
| :param powersimdata.input.grid grid: Grid instance. | ||
| :return: (*None*). | ||
| """ | ||
| if not isinstance(curtailment, dict): |
There was a problem hiding this comment.
I also recommend only validating inputs on public facing functions. If you want, you can check the inputs for private functions in tests instead. There's a lot of conflicting opinions here but in my experience most people follow these guidelines since it means a lot less work. Check it out: https://softwareengineering.stackexchange.com/questions/64926/should-a-method-validate-its-parameters/64929#64929
There was a problem hiding this comment.
What is 'public' in this case? summarize_curtailment_by_bus() and summarize_curtailment_by_location() are both public functions of the module, so I'm checking their inputs but using a custom private function to avoid duplication (since the inputs to each are the same). Is the idea that these are our own analysis functions so they're not really public? All of the _check* functions in this module are doing this: avoiding duplication of code when checking the inputs of the public functions.
There was a problem hiding this comment.
Oh, this particular function has an underscore at the beginning of the name so I'm assuming it's private.
| indexed by (datetime, plant) where plant is only plants of matching type. | ||
| """ | ||
| _check_scenario(scenario) | ||
| _check_resources(resources) |
There was a problem hiding this comment.
Like checking the resource types here is a great use case for input validation! :)
| err_msg += '. Plant: ' + ', '.join(gentypes_in_grid) | ||
| raise ValueError(err_msg) | ||
|
|
||
| def calculate_curtailment_time_series(scenario, resources=('solar', 'wind')): |
There was a problem hiding this comment.
nice job using an immutable tuple here - it avoids a lot of sneaky python bugs
postreise/analyze/curtailment.py
Outdated
| curtailment = {} | ||
| for rentype, genpotential in rentype_genpotential.items(): | ||
| ren_plants = list(genpotential.columns) | ||
| #gens_pg = pg[ren_plants] |
There was a problem hiding this comment.
think you missed deleting this
|
Should we have a curtailment time series in relative difference (in %)? The |
You raise a good point, this module does not replicate all of the functionality present in |
* feat: analyze curtailment by resource, bus * refactor: use summarize_plant_to_bus helper, clean cruft * fix: avoid error when summing subset of plants * feat: add summarization of curtailment by location, input checking * chore: cleaning up docstrings, PEP8, commented-out cruft
This branch adds a script
postreise/analyze/curtailment.pywith a few useful functions:calculate_curtailment_time_series()gives pandas DataFrames of time-series curtailment of selected resource(s), by plant.calculate_curtailment_percentage()gives the overall curtailment percentage of selected resource(s).summarize_curtailment_by_bus()gives total curtailment by resources and by bus.summarize_curtailment_by_location()gives total curtailment by resources and by (lat, long).These functions also have tests in
test_curtailment.py. Adding tests for these required adding a bit more functionality to theMockAnalyzeclass.This branch also fixes a bug in
helpers.pywhich causes angroupbyerror if you try to sum a subset of plants (since there will be a mismatch in the length of the dataframe column headers and in the the Series from the full set of plants that you're using to group by).