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

remove VRP default in VAV constructor #1561

Merged
merged 8 commits into from
Aug 11, 2023
Merged

Conversation

mdahlhausen
Copy link
Collaborator

Removes model_system_outdoor_air_sizing_vrp_method(air_loop) and air_loop_hvac_apply_vav_damper_action(air_loop) methods from the default constructor for VAV systems. Instead, default to ZoneSum for the controller mechanical ventilation, and single maximum for VAV reheat terminals. Still apply these methods before the sizing run in the create PRM baseline and create prototype methods.

Also, harmonize the outdoor air system setup between the VAV and PVAV archetypes.

Because of some slight name changes, there may be some adjustments to the regression models, though the performance results should not have changed.

Removes model_system_outdoor_air_sizing_vrp_method(air_loop) and air_loop_hvac_apply_vav_damper_action(air_loop) methods from the default constructor for VAV systems. Instead, default to ZoneSum for the controller mechanical ventilation, and single maximum for VAV reheat terminals.  Still apply these methods before the sizing run in the create PRM baseline and create prototype methods.

Also, harmonize the outdoor air system setup between the VAV and PVAV archetypes.
@mdahlhausen
Copy link
Collaborator Author

@lymereJ could you review this too? There should (hopefully) be no impact on the PRM methods.

@@ -3249,7 +3249,7 @@ def model_add_crac(model,
fan.setAvailabilitySchedule(hvac_op_sch)
else
OpenStudio.logFree(OpenStudio::Error, 'openstudio.Model.Model', "Fan type '#{fan_type}' not recognized, cannot add CRAC.")
return []
return false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like YARD comment for return should be updated.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's behind an error message, so the return type doesn't really matter. I'm aligning with the rest of the to return false after an error.

Copy link
Collaborator

@lymereJ lymereJ left a comment

Choose a reason for hiding this comment

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

If PRM tests pass, I'm fine with these changes. I just left a very minor comment.

may change to ventilation eventually
@mdahlhausen
Copy link
Collaborator Author

Thanks for the review @lymereJ! Looks like there are a few prototype regression changes, but no PRM test failures.

@mdahlhausen mdahlhausen merged commit 86bcd02 into master Aug 11, 2023
@mdahlhausen mdahlhausen deleted the fix/default_vav_controls branch August 11, 2023 22:10
@mdahlhausen mdahlhausen restored the fix/default_vav_controls branch August 15, 2023 16:57
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.

None yet

4 participants