Skip to content

Comments

Fix #4648 #4654 - SurfaceProperty:GroundSurfaces and SurfaceProperty:IncidentSolarMultiplier#177

Merged
jmarrec merged 3 commits intodevelopfrom
4648_4654_SurfaceProperties
Sep 28, 2022
Merged

Fix #4648 #4654 - SurfaceProperty:GroundSurfaces and SurfaceProperty:IncidentSolarMultiplier#177
jmarrec merged 3 commits intodevelopfrom
4648_4654_SurfaceProperties

Conversation

@jmarrec
Copy link
Contributor

@jmarrec jmarrec commented Sep 12, 2022

Pull request overview

Companion PR:

Link to the Linux.deb installer to use for CI Testing. If not set, it will default to latest official release.
[OpenStudio Installer]: http://openstudio-ci-builds.s3-website-us-west-2.amazonaws.com/PR-4658/OpenStudio-3.5.0-alpha%2B38858ac1c8-Ubuntu-18.04.deb

This Pull Request is concerning:

  • Case 1 - NewTest: a new test for a new model API class,

Case 1: New test for a new model API class

Test for SurfaceProperty:GroundSurfaces and SurfaceProperty:IncidentSolarMultiplier

Work Checklist

The following has been checked to ensure compliance with the guidelines:

  • Tests pass either:

  • Ruby test is stable: when run multiple times on the same machine, it produces the same total site kBTU.
    Please paste the heatmap png generated after running the following commands:

    • I ensured that I assign systems/loads/etc in a repeatable manner (eg: if I assign stuff to thermalZones, I do model.getThermalZones.sort_by{|z| z.name.to_s}.each do ... so I am sure I put the same ZoneHVAC systems to the same zones regardless of their order)
    • I tested stability using process_results.py (see python process_results.py --help for usage).
      Please paste the text output or heatmap png generated after running the following commands:
      # Clean up all custom-tagged OSWs
      python process_results.py test-stability clean
      # Run your test 5 times in a row. Replace `testname_rb` (eg `airterminal_fourpipebeam_rb`)
      python process_results.py test-stability run -n test_surface_properties_ground_and_solarmult_rb --os_cli=/Users/julien/Software/Others/OS-build2/Products/openstudio
      # Check that they all passed
      python process_results.py test-status --tagged
      => OK: No Failing tests were found
      # Check site kBTU differences
      python process_results.py heatmap --tagged
      => Warning: There are no percentages differences that are above the threshold=1.0000% for any test/version,but there are some non-zero values, absolute max diff is 0.1269%
  • Object has been added to autosize_hvac.rb to ensure the autosizedXXX values methods do work: N/A


Review Checklist

  • Code style (indentation, variable names, strip trailing spaces)
  • Functional code review (it has to work!)
  • Matching OSM test has been added or # TODO added to model_tests.rb
  • Appropriate out.osw have been committed
  • Test is stable
  • Object is tested in autosize_hvac as appropriate
  • The appropriate labels have been added to this PR:
    • One of: NewTest, TestFix, NewTestForExisting, Other
    • If NewTest: add PendingOSM or AddedOSM

@jmarrec jmarrec added NewTest PR type: a new test for a new model API class PendingOSM A matching OSM test has yet to be added with the next official OpenStudio Release labels Sep 12, 2022
@jmarrec jmarrec self-assigned this Sep 12, 2022
@jmarrec
Copy link
Contributor Author

jmarrec commented Sep 12, 2022

Stabilility issue?

=> Warning: There are no percentages differences that are above the threshold=1.0000% for any test/version,but there are some non-zero values, absolute max diff is 0.1269%

image

Rerunning with SAVE_IDF=True

SAVE_IDF=True python process_results.py test-stability run -n test_surface_properties_ground_and_solarmult_rb -N 10 --os_cli=/Users/julien/Software/Others/OS-build2/Products/openstudio

image

There are ZERO differences between the run 9 and the others, so I'm assuming this is an E+ problem... Here's a gist of the IDF file that is offending: https://gist.github.com/jmarrec/bd3b15c5b0272fd70e4f8d71303f5eab

@joseph-robertson joseph-robertson changed the base branch from develop to v22.2.0-IOFreeze September 16, 2022 18:19
Base automatically changed from v22.2.0-IOFreeze to develop September 19, 2022 13:05
@jmarrec jmarrec merged commit 6f49f5b into develop Sep 28, 2022
@jmarrec jmarrec deleted the 4648_4654_SurfaceProperties branch September 28, 2022 11:42
@jmarrec jmarrec added AddedOSM A matching OSM test has been added with an official OpenStudio release and removed PendingOSM A matching OSM test has yet to be added with the next official OpenStudio Release labels Jan 10, 2023
jmarrec added a commit that referenced this pull request Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AddedOSM A matching OSM test has been added with an official OpenStudio release NewTest PR type: a new test for a new model API class

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants