Skip to content

feat: improve flexibility of MW-miles calculation, for Create state and new branches#463

Merged
danielolsen merged 3 commits intodevelopfrom
daniel/improve_mwmiles_calc
Apr 27, 2021
Merged

feat: improve flexibility of MW-miles calculation, for Create state and new branches#463
danielolsen merged 3 commits intodevelopfrom
daniel/improve_mwmiles_calc

Conversation

@danielolsen
Copy link
Copy Markdown
Contributor

@danielolsen danielolsen commented Apr 23, 2021

Purpose

What the code is doing

  • In create.py and scenario.py: we add the info parameter to the Scenario (see Allow calculate_mw_miles to be used for Scenario in Create state #457 for a discussion).
  • In transform_grid.py: we move branch and DC line scaling after branch and DC line additions, to enable scaling of newly-added branches (we could equivalently scale the entries in "new_branch" and "new_dcline", but this way allows scale_congested_mesh_branches to continue to work without modifications even when it identifies that a branch not in the base grid should be upgraded). EDIT: removed. See comment: feat: improve flexibility of MW-miles calculation, for Create state and new branches #463 (comment)
  • In mwmiles.py: we make use of the TransformGrid class to be able to easily calculate differences in branch rateA for both scaled and new branches, and to automatically look up branch types, lats, and lons.
  • In test_mwmiles.py: we add impedances to the fake data so that TransformGrid works properly.

Testing

All unit tests still pass, and calculations work as expected on Scenarios with new branches (see 3822 or 3828 for examples). Testing on previously-analyzed scenarios with lots of branch upgrades (existing branches only) still produces identical results.

Time estimate

15-30 minutes.

@danielolsen danielolsen added the new feature Feature that is currently in progress. label Apr 23, 2021
@danielolsen danielolsen self-assigned this Apr 23, 2021
@BainanXia
Copy link
Copy Markdown
Collaborator

I have two questions regarding the logic:

  1. When adding new branches, what if the user doesn't want it be scaled given the Pmax is part of user's inputs, assuming we have branch scaling in a zone (currently we never did that though).
  2. scale_congested_meshed_branches will never identify a branch not in the "base_grid" right? It takes ref_scenario as parameter, in which case the base_grid is the grid of the reference scenario. Any branches not in the grid of reference scenario can't be congested either, since the simulation hasn't been run yet.

@danielolsen
Copy link
Copy Markdown
Contributor Author

I have two questions regarding the logic:

1. When adding new branches, what if the user doesn't want it be scaled given the `Pmax` is part of user's inputs, assuming we have branch scaling in a zone (currently we never did that though).

2. `scale_congested_meshed_branches` will never identify a branch not in the "base_grid" right? It takes `ref_scenario` as parameter, in which case the `base_grid` is the grid of the reference scenario. Any branches not in the grid of reference scenario can't be congested either, since the simulation hasn't been run yet.

For (1), I agree, maybe we want to keep TransformGrid the way it is so that there is no confusion where the user specifies one branch capacity and then it gets zone-scaled. I remember conversations with you and @rouille along those same lines for plants.

For (2), scale_congested_meshed_branches can identify any branch in a Grid from a previously-run scenario. We may have added a branch, run the scenario to see where the congestion is, and then we want to upgrade some branches, one of which is one that is not present in the base grid. I just ran into that situation myself. We can implement this logic without changing TransformGrid by modifying powersimdata.design.transmission.upgrade.increment_branch_scaling so that it can upgrade added branches as well using similar logic, once they're identified in powersimdata.design.transmission.upgrade._identify_mesh_branch_upgrades.

@danielolsen danielolsen force-pushed the daniel/improve_mwmiles_calc branch 3 times, most recently from 89e9555 to ec8590f Compare April 26, 2021 15:17
@danielolsen
Copy link
Copy Markdown
Contributor Author

I've removed the re-ordering of the TransformGrid so that this PR can be more focused. I have follow-up ideas about the TransformGrid, but I will save them for a separate issue.

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.

Looks good.

@danielolsen danielolsen force-pushed the daniel/improve_mwmiles_calc branch 2 times, most recently from b7b3b74 to 5fc4afd Compare April 26, 2021 17:15
upgrades["num_transformers"] += 1
else:
raise Exception("Unknown branch: " + str(b))
raise Exception("Unknown branch type: " + str(b))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it be helpful to include the unknown branch type device_type in the error message and clarify that b is the branch id?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This else is more for debugging than anything else, since the code should never in normal operation get here. There are no branch device types in our current grid model that are not covered by these three options.

upgraded_branch_ids = set(ct["branch"]["branch_id"].keys())
transformed_branch = TransformGrid(original_grid, ct).get_grid().branch
if "new_branch" in ct:
upgraded_branch_ids |= set(transformed_branch.index)
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.

Am I reading this correctly - we could technically use upgraded_branch_ids = set(transformed_branch.index) to begin with, but in the case where there are no new branches, it's easier to just grab the scaled branch ids from the change table since there will be fewer to consider?

Copy link
Copy Markdown
Contributor Author

@danielolsen danielolsen Apr 26, 2021

Choose a reason for hiding this comment

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

Good catch, I made a mistake when simplifying something here. We don't want the full transformed branch index, we want the entries in the change table plus any new entries, which is the difference between the transformed branch and the base one. I've pushed a commit to fix.

@danielolsen danielolsen force-pushed the daniel/improve_mwmiles_calc branch from 2877995 to c599b9f Compare April 27, 2021 14:01
@danielolsen danielolsen merged commit 56aea61 into develop Apr 27, 2021
@danielolsen danielolsen deleted the daniel/improve_mwmiles_calc branch April 27, 2021 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new feature Feature that is currently in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Include new branches in calculations of MW-miles of upgrades Allow calculate_mw_miles to be used for Scenario in Create state

4 participants