-
Notifications
You must be signed in to change notification settings - Fork 40
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
feat: mesh branch upgrade priorities #120
Conversation
@@ -163,11 +165,20 @@ def _identify_mesh_branch_upgrades(ref_scenario, upgrade_n=100, quantile=0.95): | |||
scenario to be used to determine the most congested branches. | |||
:param int upgrade_n: the number of branches to upgrade. | |||
:param float quantile: the quantile to use to judge branch congestion. | |||
:param str method: prioritization method: 'branches', 'MW', or 'MWmiles'. | |||
:return: (*Set*) -- A set of ints representing branch indices. |
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.
Set --> set in docstring
@@ -1,6 +1,7 @@ | |||
import numpy as np | |||
import pandas as pd | |||
|
|||
from postreise.analyze.mwmiles import haversine |
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.
2 blank lines (PEP8)
allowed_methods = ('branches', 'MW', 'MWmiles') | ||
if method not in allowed_methods: | ||
allowed_list = ', '.join(allowed_methods) | ||
raise ValueError('method must be one of: {allowed_list}') |
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.
variable allowed_list
is not used. 'method must be one of: {%s}' % allowed_list
should work
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.
Good catch, I had meant to use f-strings, which does the same thing (in my opinion in a cleaner format).
ValueError(f'method must be one of: {allowed_list}')
l.63 in docstring, :param pandas.DataFrame branch: branch DataFrame from Grid object. --> :param pandas.DataFrame plant: plant DataFrame from Grid object. |
Is there any reason for |
@rouille I think I see the public vs. private issue. I intended this whole module to be invoked via |
All style/doc changes are addressed, and the public function has been updated to pass through the method parameter. Better demo of the function as the user will interact with it:
|
31654b9
to
d42e64a
Compare
The branch has now been interactive rebased to what I think are only the essential commits. |
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.
Great new feature
Purpose
This PR adds a feature to be able to choose how to prioritize which congested mesh branches to upgrade. Previously the ranking was always by average congestion shadow price, now there are also options for [average congestion shadow price per MW that will be upgraded] and [average congestion shadow price per MW-mile that will be upgraded]. For zero-length branches the length is given as 1 mile.
Closes #116
What is the code doing
The function
_identify_mesh_branch_upgrades
now takes one more parameter,method
, which must be one of('branches', 'MW', 'MWmiles')
(defaults to'branches'
). Once the average congestion shadow price is determined, the metric by which the branches will be ranked is calculated in one of three ways (specified by method), and branches are then ranked by this metric, with the top N branches returned.Tests for the new ranking methods are given in
test_design_transmission.py
, where the test case is engineering to produce different rankings for the three methods.How to test the code
IMPORTANT: This code requires that you have the following fix applied to PostREISE: Breakthrough-Energy/PostREISE#84
Demo:
Overall the rankings for this scenario are similar, but MWmiles gives a different result than MW or branches.
Time to review
Estimated 1 hour or less. There's about 30 lines of real useful code, the rest is tests.