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

Fix UnmetLoads measure for OS 3.0 and newer #168

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

shorowit
Copy link

@shorowit shorowit commented May 2, 2024

Fixes #76

@shorowit shorowit added the bug Something isn't working label May 2, 2024
@shorowit shorowit self-assigned this May 2, 2024
@shorowit
Copy link
Author

shorowit commented May 2, 2024

@DavidGoldwasser I made the change and re-enabled the tests, but two of the tests are failing. They seem unrelated to my change, so I will let you decide what to do about them. I'm not familiar enough with the measure to know what's going on.

@shorowit shorowit requested a review from DavidGoldwasser May 2, 2024 17:17
Comment on lines -261 to +262
exit_temp = plantloop.sizingPlant.getDesignLoopExitTemperature.value
exit_temp = plantloop.sizingPlant.designLoopExitTemperature
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Primary bugfix

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -443,6 +444,8 @@ def time_series_setpoint_vs_temp(zoneMetrics)
@test_nine_data << graph
end
end

attr_reader :measureMetrics
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed for the measure unit tests to work

Comment on lines 160 to 105
<identifier>2.0.0</identifier>
<min_compatible>2.0.0</min_compatible>
<identifier>3.0.0</identifier>
<min_compatible>3.0.0</min_compatible>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bumped min version

Comment on lines +7 to +8
require 'openstudio'
require 'openstudio/measure/ShowRunnerOutput'
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-enabled all tests (but two still fail).

@aaron-boranian
Copy link

@DavidGoldwasser @shorowit
Just came across this issue with Unmet Load Hours troubleshooting measure while prepping for an OpenStudio Application training workshop. In the meantime, I can create a custom version of this measure to share with workshop attendees.

It looks like Scott's pull request had some merge conflicts to resolve.

@DavidGoldwasser
Copy link
Collaborator

@aaron-boranian Thanks for pinging this. I'll look at merge conflicts and see if I can get tests passing and this merged

@DavidGoldwasser DavidGoldwasser added this to the Measures For OS 3.9.0 milestone Sep 24, 2024
@DavidGoldwasser
Copy link
Collaborator

Not sure why I'm getting failures or errors in 7 measures when only one was changed. Try CI run of clean develop see what is happening, and run locally.

image

@DavidGoldwasser DavidGoldwasser self-assigned this Sep 25, 2024
@DavidGoldwasser
Copy link
Collaborator

I started to look at this hoping to get in 3.9.0 but not sure if I'll get it in time. First I remember whey the test was commented out, it overwrites some OpenStudio methods like OpenStudio::SqlFile that break other measures that use that method that run after this.

This was one of first reporting measures and was using pre-made SQL file which was fine at the time, but now all reporting measures run a simulation and make a fresh SQL file as part of the test. I'll update this measure to do that but may not have time to do it in time to release with update for OpenStudio 3.9.0.

Another thing I noticed is the test SQL file results in 0 unmet hours which makes it hard to see much. That may be because it is old SQL file not being read correctly. Maybe will have more populated HTML with a new model.

Screenshot 2024-11-08 at 3 17 55 PM

@DavidGoldwasser DavidGoldwasser marked this pull request as draft November 8, 2024 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

UnmetLoads measure needs to be updated for OSv3
4 participants