-
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
fix: ensure that get_bus_demand works in Create state #422
Conversation
@@ -149,6 +149,7 @@ def get_bus_demand(self): | |||
|
|||
:return: (*pandas.DataFrame*) -- data frame of demand (hour, bus). | |||
""" | |||
self._update_scenario_info() |
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.
It makes sense. I am confused to why it was working before without the update.
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.
get_demand
uses get_profile
which uses self.builder.demand
, but get_bus_demand
passes self.scenario_info
without updating self.scenario_info
with what we're setting in the builder.
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.
When the bug has been introduced because get_bus_demand
was working originally.
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.
Did we test it in Create state after #388?
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.
Probably not but I thought this was a feature you were using before creating each scenario. That's why I am surprised you just found the bug now.
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.
Nope, this method was not part of my usual scenario setup process. REISE.jl does the same thing within the Julia code though. This feature was mostly being used for scenarios in Analyze state.
bus = grid.bus | ||
demand = InputData().get_data(scenario_info, "demand")[bus.zone_id.unique()] |
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 safeguard
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!
…s_demand fix: ensure that get_bus_demand works in Create state
Purpose
Fixes a couple of bugs when trying to get bus_demand from a Scenario in Create state. Closes #421.
What the code is doing
scenario_info
gets updated before we callget_bus_demand
.Testing
The automated tests don't cover this, because getting the demand profiles requires ssh, but you can check that tests pass locally using
python -m pytest
.Time estimate
5 minutes.