Conversation
e24c00f to
155fa45
Compare
|
I relaxed the Pmax = 0 check because it was complaining about a few hydro plants with 0 capacity in Montana Eastern. Here are the new warnings: |
|
@BainanXia, I refactored the warning logic a bit in this last commit, now it should only warn if scaling for a given plant == 1 when considering both |
| if stub_degree > 0: | ||
| ren_capacity = _find_capacity_at_bus(ref_plant, bus_id, r) | ||
| assert ren_capacity > 0 | ||
| try: |
There was a problem hiding this comment.
Reading through the code to understand the logic. Is there a specific reason that we are not using if/else instead of try, assert, except AssertionError?
There was a problem hiding this comment.
...only because it was an assert before, and this was a quick fix. I agree, your proposal is cleaner, I will change it.
| except KeyError: | ||
| pass | ||
| if (gen_scale_factor == 1) and (verbose == True): | ||
| print(f'no scaling factor for {r}, {zone_id}, plant {p}') |
There was a problem hiding this comment.
I'm trying to understand whether this is a common situation or not, in other words, how often is a plant of type 'r' not being scaled neither via 'zone_id' nor 'plant_id', given you are looping through all the plants of such type.
There was a problem hiding this comment.
I guess that depends on the scenario in question. In our Eastern 2030 Independent OB1, when we are loading capacities from a change table and then turning off offshore_wind, we have scaling factors for all plants/types. But I can imagine other scenarios which would not have scale factors for all types, where we would maybe still run renewable stub scaling.
There was a problem hiding this comment.
Then in such scenario we are going to have a lot of such warnings for each plant of 'r'+'zone_id'.
There was a problem hiding this comment.
Maybe it makes more sense to turn verbose off by default instead of on? The user can always specify from the change_table.scale_renewable_stubs() call whether they want these warnings or not.
There was a problem hiding this comment.
Agree. Otherwise user like me would be anxious in seeing so many warnings and suspecting something seriously is going wrong.
There was a problem hiding this comment.
Done, with an update to the docstring as well.
|
Let me confirm with you regarding the logic
Does this happen after we reset the Pmax of hydro generators according to the maximum of the corresponding profiles, which implies we have a bunch of 'dummy' hydro plants with zero capacities and all zero profiles? |
Unfortunately, yes. I think all these generators were scaled down to zero when we rebased from our Eastern cost curve tuning process, so the Pmax is 0 and the profiles are |
|
@danielolsen I don't remember we ever turned any hydro plants from TAMU network down before. Is it because they shouldn't there according to the external references and we turn them off by scaling capacities of those plants down to zero? Or, they come with zero capacity generators from TAMU network originally. |
|
@BainanXia when we were running cost curve tests, we scaled plants based on excel spreadsheets, and the entry for hydro in Montana was Then we rebased the grid based on one of those scenarios. |
|
@danielolsen I see. RIP for those hydro plants. It is what it is. Probably in the future when we are cleaning up the grid, we could erase them together with those dummy branches. |
BainanXia
left a comment
There was a problem hiding this comment.
Thanks for walking through the logic.
02336c0 to
9cf58b9
Compare
Purpose
Never again have to manually scale stub branches to deal with hydro. This is in conjunction with #162, to get accurate Pmax values for hydro plants into the base grid.
What is the code doing
One line change, adding
'hydro'to the list of renewable plant types to perform renewable stub scaling on.Time to review
5 minutes.