Refactor Schedules module #1656
Conversation
schedule_constant_annual_equivalent_full_load_hrs -> schedule_constant_get_equivalent_full_load_hours get_8760_values_from_schedule_constant -> schedule_constant_get_hourly_values note that the get_8760_values_from_schedule_constant method added an extra 24 hours for holiday schedules, but those extra hours are never used in the code. It's only used with ScheduleRuleset returns.
schedule_compact_annual_min_max_value -> schedule_compact_get_min_max add generic schedule_get_min_max method
schedule_compact_design_day_min_max_value -> schedule_compact_get_design_day_min_max
day_schedule_equivalent_full_load_hrs -> schedule_day_get_equivalent_full_load_hours schedule_ruleset_design_day_min_max_value -> schedule_ruleset_get_design_day_min_max schedule_ruleset_annual_equivalent_full_load_hrs -> schedule_ruleset_get_equivalent_full_load_hours + schedule_get_design_day_min_max + schedule_get_equivalent_full_load_hours + schedule_constant_get_design_day_min_max - removed old tests for schedule ruleset methods simplified some sections of code with switch statements depending on schedule type to the generic schedule_get method
schedule_ruleset_annual_hours_above_value -> schedule_ruleset_get_hours_above_value also fixed method because it was giving total eflh, not the sum of hours above a value
replace two methods: schedule_ruleset_annual_hourly_values -> schedule_ruleset_get_hourly_values get_8760_values_from_schedule_ruleset -> schedule_ruleset_get_hourly_values both of the methods failed the tests to get the proper values. schedule_ruleset_annual_hourly_values averages values at 15, 30, 45, which will improperly weight value changes not at 15 min intervals. get_8760_values_from_schedule_ruleset assumed schedules have fixed intervals, which likely won't be true in cases of splines or ramps for custom parametric schedules the new method schedule_ruleset_get_hourly_values uses the smallest time interval in a day schedule to calculate the averages, which is more accurate. calls to the old methods replaced with the generic schedule_get_hourly_values additionally: - get_8760_values_from_schedule + schedule_day_get_hourly_values + schedule_get_hourly_values
model_add_schedule_type_limits -> create_schedule_type_limits model_add_constant_schedule_ruleset -> create_constant_schedule_ruleset
|
@weilixu I'd appreciate your review of the changes in this PR that impact PRM methods. In particular, there are several PRM methods that get hourly values. I also made a change to how hourly values are calculated, which is in the change from |
There was a problem hiding this comment.
Thank you @mdahlhausen That was a lot work to make the repo organized and improved!
I have a few comments as suggestions. I think your changes make sense and do not see any major impact to PRM routine.
| year_end_date = nil | ||
| if schedule_ruleset.model.yearDescription.is_initialized | ||
| year_description = schedule_ruleset.model.yearDescription.get | ||
| year = year_description.assumedYear |
There was a problem hiding this comment.
We actually had an internal discussion on the assumedYear. Does this assumedYear guaranteed a non-leap year?
The reason is the PRM guideline requires PRM run in a non-leap year, so we may need some functions to make sure we are not running in leap year simulation.
There was a problem hiding this comment.
It does not guarantee a non-leap year. How about at the start of the PRM methods, adding a check for the year, and setting to a default non-leap year if it is a leap year?
There was a problem hiding this comment.
@weilixu looking more at the PRM code, there are a few places that use schedule_get_hourly_values. It seems those places specifically loop through weeks and hours to get exactly 8760 values, get_fan_hours_per_week for example:
def get_fan_hours_per_week(model, air_loop)
fan_schedule = air_loop.availabilitySchedule
fan_hours_8760 = OpenstudioStandards::Schedules.schedule_get_hourly_values(fan_schedule)
fan_hours_52 = []
hr_of_yr = -1
(0..51).each do |iweek|
week_sum = 0
(0..167).each do |hr_of_wk|
hr_of_yr += 1
week_sum += fan_hours_8760[hr_of_yr]
end
fan_hours_52 << week_sum
end
max_fan_hours = fan_hours_52.max
return max_fan_hours
end
I'm wondering if we need to make any changes if the PRM methods that use the methods all force 8760 values anyways.
There was a problem hiding this comment.
to be safe, it seems the best way forward is to fix the year to a non-leap year if the AssumedYear is a leap year.
Pull request overview
This PR moves several schedules methods from:
to the
OpenstudioStandards::SchedulesmodulePull Request Author
bundle exec rake doc)bundle exec rake rubocop)Review Checklist
This will not be exhaustively relevant to every PR.