Skip to content
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

Merge nrcan changes into master #1721

Merged
merged 30 commits into from
Apr 7, 2024
Merged

Merge nrcan changes into master #1721

merged 30 commits into from
Apr 7, 2024

Conversation

ckirney
Copy link
Collaborator

@ckirney ckirney commented Apr 5, 2024

Pull request overview

Merging changes and fixes in nrcan branch into master. Also, updating nrcan test results regarding cop changes and changes from move to OS 3.7.0.

  • Fixes #ISSUENUMBERHERE (IF THIS IS A DEFECT)

Pull Request Author

  • Method changes or additions
  • Data changes or additions
  • Added tests for added methods
  • If methods have been deprecated, update rest of code to use the new methods
  • Documented new methods using yard syntax
  • Resolved yard documentation errors for new code (ran bundle exec rake doc)
  • Resolved rubocop syntax errors for new code (ran bundle exec rake rubocop)
  • All new and existing tests passes
  • If the code adds new require statements, ensure these are in core ruby or add to the gemspec

Review Checklist

This will not be exhaustively relevant to every PR.

  • Perform a code review on GitHub
  • All related changes have been implemented: method additions, changes, tests
  • Check rubocop errors
  • Check yard doc errors
  • If fixing a defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If a new feature, test the new feature and try creative ways to break it
  • CI status: all green or justified

ckirney and others added 25 commits January 18, 2024 11:26
* removing PERFORM_STANDARDS variable

* Add UEF to EF conversion.

* Missing argument.

* Remove simple glazing constructions and adapt code to handle it; Picking up Rubocop fixes in other files.

* Fix regex issue.

* Update material name in constructions.

* Ok, this time last missing renaming to do.

* Address issues with arguments.

* Add tests, misc formatting changes.

* Fix typos.

* Missing argument.

* Remove unecessary assertion.

* Update performance test results.

* Update regression models.

* Fix typo.

* use descriptive water heater variable names

rubocop suggested changing ef to energy _factor.

descriptive name changes:
ef -> energy_factor
uef -> uniform_energy_factor
water_heater_eff -> water_heater_efficiency
re -> recovery_efficiency

* replace schedule_ruleset_annual_min_max_value

* replace schedule_constant_annual_min_max_value

* move Standards.ScheduleConstant.rb methods

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.

* replace schedule_compact_annual_min_max_value

schedule_compact_annual_min_max_value -> schedule_compact_get_min_max
add generic schedule_get_min_max method

* replace schedule_compact_design_day_min_max_value

schedule_compact_design_day_min_max_value -> schedule_compact_get_design_day_min_max

* fix method call error in prm test helper

* replace variable public_send with case statement

* move schedule ruleset design day min max and eflh

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

* stub parametric schedules file

* OpenstudioStandards::Schedules not OpenStudioStandards::Schedules

* move schedule_ruleset_annual_hours_above_value

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

* override area for failing ffactor cons

* add schedule_get_hourly_values

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

* Update prm_test_helper.rb

* Update prm_test_helper.rb

* add pvav gas enumerations

* refactor Standards.Model schedule methods

model_add_schedule_type_limits -> create_schedule_type_limits
model_add_constant_schedule_ruleset -> create_constant_schedule_ruleset

* Add error checking, then check library. Add unit test.

* Don't add U/SHGC/VT props to the name of SG materials.

* Perf tests.

* fix keywork arg call

* Update regression models.

* Update ashrae_90_1_prm.SpaceType.rb

space_type_properties['lighting_space_type'] should be space_type_properties['lpd_space_type']

* add module descriptions for YARD docs

* minor rubocop fixes

* fix misspelling intialize -> initialize

* Update space_type_blend.rb

* add guard clauses to schedule_ruleset methods

* enforce PRM model to have a non-leap year

* rubocop edit to log message

* fix incorrect vestibule floor area check

* Enabling include statements and gems used by nrcan.

* Removing redundant hdd for climate data reporting.

* Resolving some formatting issues and updating weather test results to inculde case when weather download test is not run first.

---------

Co-authored-by: Macdonald, Iain <iain.macdonald@nrc-cnrc.gc.ca>
Co-authored-by: Lerond, Jeremy <jeremy.lerond@pnnl.gov>
Co-authored-by: Jeremy L <5149279+lymereJ@users.noreply.github.com>
Co-authored-by: Matthew Dahlhausen <matthew.dahlhausen@gmail.com>
Co-authored-by: Eric Ringold <ericringold@gmail.com>
…e an error when a storey has no spaces associated with it.
Incorporating natural ventilation fix from nrcan branch.
Merging nrcan_3.7.0 into nrcan branch
…ed an estimate for the pipe diameter of ~0.019 m. This value results in very large velocities through the pipes and a very large estimate for the pump power. This approach is replaced with using a recommended value for the maximum velocity through the pipes and then finding an appropriate pipe diameter.
# Conflicts:
#	data/standards/test_performance_expected_dd_results.csv
@ckirney ckirney requested a review from mdahlhausen April 5, 2024 19:28
@mdahlhausen mdahlhausen changed the title Merge nrcan chnages into master Merge nrcan changes into master Apr 5, 2024
The BTAP geometry wizard no longer exists; these tests were moved to modules/geometry/test_geometry_create_shape.rb
@ckirney
Copy link
Collaborator Author

ckirney commented Apr 5, 2024

I updated the regression tests so they should work now. However, I found that the 'test_NECB2020_shw_gas_efficiency_standby_losses' test in 'test_necb_waterheater_rules.rb' fails. Specifically here:

run_sizing(model: model, template: template, test_name: name, save_model_versions: save_intermediate_models)

which ultimately fails here:

most_used_profile = profile_days_hash.max_by { |k, v| v.size }.first

I believe this has to do with the changes from PR# 1718 (#1718). Unfortunately, I won't have time to look into further until late next week or later. I'd rather fix this issue but I suppose we can skip the test for now and I can look into it later when I have more time.

@mdahlhausen
Copy link
Collaborator

Thanks @ckirney. The issue you noted was introduced in #1718. spaces_get_occupancy_schedule would error whenever there were no people in the spaces. I fixed it with #1722.

Thanks for updating all the models! Should have a passing CI again soon.

@mdahlhausen mdahlhausen merged commit e7d43f4 into master Apr 7, 2024
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants