-
Notifications
You must be signed in to change notification settings - Fork 23
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 stochastic schedule generation runtime #1933
Conversation
@schedules[SchedulesFile::Columns[:HotWaterFixtures].name] = fixtures.map { |flow| flow / fixtures.max } | ||
@schedules[SchedulesFile::Columns[:HotWaterFixtures].name] = normalize(fixtures) |
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.
This is the primary bugfix. fixtures.max
was being calculated within the iteration resulting in a huge performance hit.
discharging_schedule += [1] * first_half_driving # Start driving | ||
discharging_schedule += [0] * idle_time # Idle in the middle | ||
discharging_schedule += [1] * second_half_driving # End driving | ||
charging_schedule += [0] * activity_minutes | ||
discharging_schedule.concat([1] * first_half_driving) # Start driving | ||
discharging_schedule.concat([0] * idle_time) # Idle in the middle | ||
discharging_schedule.concat([1] * second_half_driving) # End driving | ||
charging_schedule.concat([0] * activity_minutes) | ||
|
||
driving_minutes_used += actual_driving_time | ||
else | ||
charging_schedule += [1] * activity_minutes | ||
discharging_schedule += [0] * activity_minutes | ||
charging_schedule.concat([1] * activity_minutes) | ||
discharging_schedule.concat([0] * activity_minutes) |
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.
Specifically for EVs, this code was also resulting in some slowdown since +=
creates new arrays and all of this is happening within a loop.
hours_per_year = (vehicle.hours_per_week / 7) * UnitConversions.convert(1, 'yr', 'day') | ||
hours_per_week = get_ev_hours_per_week(vehicle) |
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.
Found a bug while working on all of this. hours_per_week
is not a required input, so we need to default it when not provided.
I should see if we can apply defaults to the entire HPXML Building (or a copy of the object, if we need to prevent writing the defaults out to disk) to generically prevent issues like this in the future...
…vidual defaulting methods. Should help prevent future bugs like vehicle.hours_per_week.
CI results:
I'm calling it good enough. |
Pull Request Description
Fixes a regression introduced here that resulted in very slow generation of stochastic schedules.
Checklist
Not all may apply:
EPvalidator.xml
) has been updatedopenstudio tasks.rb update_hpxmls
)HPXMLtoOpenStudio/tests/test*.rb
and/orworkflow/tests/test*.rb
)openstudio tasks.rb update_measures
has been run