Skip to content
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: recover plant and branch indices #94

Merged
merged 11 commits into from
Jun 3, 2021

Conversation

BainanXia
Copy link
Collaborator

Pull Request doc

Purpose

Recover plant and branch indices from Switch outputs as discussed in thread, which closes #91

What the code is doing

  • Rename the current make_indices function in helpers.py to make_plant_indices.
  • Create a new function recover_plant_indices in helpers.py to deserialize plant indices that are serialized in make_plant_indices.
  • Create a new function make_branch_indices that takes care of aclines and dclines.
  • Refactor build_aclines, build_dclines in grid_to_switch.py using make_branch_indices.
  • Create a new function recover_branch_indices in helpers.py to deserialize branch indices that are serialized in make_branch_indices.
  • Add corresponding tests.

Testing

Tests for recovery functions are added. Rest of the code is tested manually.

Where to look

switchwrapper/helpers.py
switchwrapper/tests/test_helpers.py

Time estimate

15-20 minutes. There might be some back and forth modifications given these functions are related to #49 #50.

@BainanXia BainanXia self-assigned this Jun 2, 2021
@danielolsen
Copy link
Contributor

It would be nice if we had an end-to-end test of using the functions to encode our indices to Switch indices, and then decode the original indices. That way we could ensure that if we ever change e.g. make_plant_indices or recover_plant_indices, we will know that they are still compatible.

The current code looks good though, I will branch off of here and start working on #49, and let you know if I run into any problems.

@BainanXia
Copy link
Collaborator Author

It would be nice if we had an end-to-end test of using the functions to encode our indices to Switch indices, and then decode the original indices. That way we could ensure that if we ever change e.g. make_plant_indices or recover_plant_indices, we will know that they are still compatible.

The current code looks good though, I will branch off of here and start working on #49, and let you know if I run into any problems.

I was thinking about an end-to-end test as well. However, what I did manually is simply run make_***_indices fed with grid object, run recover_***_indices, then compare. Apparently, this check can't make sure there is no "compatibility" issue that you might encounter when dealing with #49. Thus, I decided to keep this open to handle any potential adjustments along the way until I get a green light from #49.

@BainanXia BainanXia changed the title Recover plant and branch indices feat: recover plant and branch indices Jun 3, 2021
@@ -8,3 +11,70 @@ def make_indices(plant_ids):
original_plant_indices = [f"g{p}" for p in plant_ids]
hypothetical_plant_indices = [f"{o}i" for o in original_plant_indices]
return original_plant_indices, hypothetical_plant_indices
Copy link
Collaborator

@rouille rouille Jun 3, 2021

Choose a reason for hiding this comment

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

Sorry, I guess I missed the original PR implementing this function. If I understand correctly, each plant in the input grid can be scaled by Switch since len(riginal_plant_indices) == len(hypothetical_plant_indices). What happens if only a handful are scaled? Do we have only the ones that has been updated in the output file? Or the original along with all the hypothetical ones with value for the latter reflecting an update will be in the output file?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good question! I'm wondering the same thing. My presumption is all plants (original + hypothetical) are in the output file and the values will reflect an upgrade or not. @YifanLi86 will have a better idea on this.

Copy link
Contributor

Choose a reason for hiding this comment

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

For the generation build decisions, I believe that Switch outputs the value of each variable, even if that value is zero (e.g. most generators are not expanded). So the output file include both build 'decisions' for '2019' (the predetermined build representing the current grid) and build decisions for the investment year(s).

Copy link
Contributor

@YifanLi86 YifanLi86 Jun 3, 2021

Choose a reason for hiding this comment

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

@BainanXia Indeed good point. For all original + hypothetical units there will be DispatchGen in output, but there will be BuildGen results if and only if it is a candidate (i.e., there is build cost been assigned to it), even if the expansion is 0 I believe.

SWITCH uses the logic of generation expansion in a previous year to be able to assign existing generation, rather than putting "permanent" existing gen capacity there directly lasting forever. So we have to put all units exist in gen_build_predetermined.csv in the gen_build_cost.csv, otherwise SWITCH will give errors.

This actually reminds me a question: so like a lot of expansion model, SWITCH tells if a unit is an expansion candidate by looking for its gen cost property. If there is gen build cost assigned, it will treat it as expansion candidate and there will be BuildGen in pickle. So shall we actually add this logic in our candidate input as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@YifanLi86 I'm not quite sure I understand your question but it seems to be interesting. If you don't mind, could you create an issue for that given it is out of the scope of this PR, then we could follow up that track?

Copy link
Contributor

@danielolsen danielolsen left a comment

Choose a reason for hiding this comment

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

The code looks good (with tests!), and I have successfully (I think) used the helper functions in extraction of an expanded Grid in my branch daniel/make_output_grid. I think this is good to merge into output_processing after a rebase. We may identify some changes that we want to make once we are reviewing the final Grid and time-series extractions, but that shouldn't hold up getting this into the branch so that we can start our development/testing on those other features.

@BainanXia BainanXia force-pushed the bainan/recover_indices branch from 235c277 to 8f780e7 Compare June 3, 2021 20:53
@BainanXia BainanXia merged commit 3a0d577 into output_processing Jun 3, 2021
@BainanXia BainanXia deleted the bainan/recover_indices branch June 3, 2021 20:55
danielolsen pushed a commit that referenced this pull request Jun 3, 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