-
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
Port linearize_gencost and pmin modifications from REISE.jl #701
Conversation
Is powersimdata/design/generation/ a good location for the |
I agree. The original idea of |
powersimdata/scenario/execute.py
Outdated
def _adjust_pmin(self): | ||
mi = self.grid.model_immutables | ||
plant = self.grid.plant | ||
pmin_factor = mi.plants["pmin_as_share_of_pmax"] | ||
|
||
def _scale(x): | ||
factor = pmin_factor.get(x.type) | ||
return x.Pmin if factor is None else factor * x.Pmax | ||
|
||
plant.Pmin = plant.apply(_scale, axis=1) | ||
|
||
# set Pmin to 0 for generators that are off or profile based | ||
profile_resource = list(mi.plants["profile_resources"]) | ||
plant.loc[plant.type.isin(profile_resource), "Pmin"] = 0 | ||
plant.loc[plant.status == 0, "Pmin"] = 0 |
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.
Maybe out of the scope of this PR, two high level questions:
hydro
is one ofprofile_resources
, thePmin
of which will be set 0 at the end, however thepmin_factor
ofhydro
is grid dependent .- We will need to build user interface to pass user specified
pmin_as_share_of_pmax
(either plant based or plant type based or plant type * zone based) in the future, right?
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.
- resources with profile have all
Pmin
set to 0 in theplant
data frame but it is not used in the optimization. We did that for backward compatibility since this is what was hard coded in REISE.jl. - If we want a finer resolution on
pmin_as_share_of_pmax
, we can have either an extra column in theplant
data frame or a dedicated data structure than can be passed to the engine. Either way we will need a user interface to specify the values
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.
- True. Just pointing out such entries are modified twice in the current process.
- Agreed.
def test_linearize_gencost_pmin_equal_pmax(): | ||
plant = mock_grid_gc.plant.copy() | ||
plant.Pmin = plant.Pmax | ||
grid = MockGrid({"plant": plant.reset_index().to_dict(), "gencost_before": mock_gc}) | ||
actual = linearize_gencost(grid, num_segments=3) | ||
assert_frame_equal(expected_all_equal, actual, check_dtype=False) |
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.
Thanks! We thought about adding such tests a while ago as in the very first version of implementation, dividing by zero wasn't avoided.
Is the ramp module essential? Especially once (if?) |
We will need that when scaling existing plants as well as adding new plants without specifying ramp rates until we have a better solution. |
It can be done in a separate PR. I am fine with having the |
Based on the current package structure, I think having those in |
a1b54c9
to
152c5b5
Compare
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.
Looks good to me
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.
Thank you for tackling this!
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.
Nice 🚀
Purpose
To facilitate using a grid.pkl and remove the need for mapping between a case.mat/grid.mat/etc, we move grid modifications previously done in REISE.jl to the scenario preparation process.
This PR should be merged at the same time as Breakthrough-Energy/REISE.jl#187, after which we should update the existing grid.mat files. I believe we also need to update the ramp30 values in plant.csv as discussed in this issue #680 and release a new dataset on zenodo.
What the code is doing
Scale plant Pmin values and linearize cost curves the same way we were doing in REISE.jl - after we apply the change table, and just before we upload the grid.pkl to the server (or, container, or wherever). I also ported the linearize_gencost tests from julia since those will be unused/deleted.
Testing
Ran a scenario in a container and reloaded it after it finished for some cursory spot checking.
Time estimate
20 min