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

Add dedicated secondary loop for 90.1 PRM plant loops #1760

Merged
merged 14 commits into from
Sep 4, 2024

Conversation

jugonzal07
Copy link
Collaborator

@jugonzal07 jugonzal07 commented Jun 12, 2024

Pull request overview

This pull implements dedicated secondary loops for the ASHRAE 90.1 PRM plant loops that utilize constant primary, variable secondary configurations. This required updating the hardcoded value that skipped all the secondary loop logic, correctly implementing staging using EMS for the inlet pumps to each chiller, and updating the operation scheme to match the new logic. Allows for adding up to 3 chillers with logic in place to catch scenarios when more than 3 are added to a loop to consolidate them.

Pull Request Author

  • Method changes or additions
  • Added tests for added 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

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

@jugonzal07 jugonzal07 added the OS Prototypes Improvements to the OS prototype models. label Jun 12, 2024
@jugonzal07 jugonzal07 requested review from lymereJ and weilixu June 12, 2024 15:22
@weilixu weilixu added AppendixG Methods to enable the Appendix G model workflow and removed OS Prototypes Improvements to the OS prototype models. labels Jun 12, 2024
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.

Thanks for that! I think that it overall looks good. I'm leaving a few comments/questions below. Also, I think that it would be nice to include a PRM test to make sure that the correct modeling inputs are added. It would be nice to also somehow have a test to check that the pumps/chillers are correctly staged but this might a bit out of scope for now.

gonz102 and others added 6 commits July 17, 2024 19:35
… the chillers by reference capacity and revert back to old naming convention.
…ities from the sizing run. This happens if there's no compressor type defined and it was causing issues in a handful of test cases.
…The rest will use the legacy common_pipe configuration once again.
@weilixu
Copy link
Collaborator

weilixu commented Aug 22, 2024

@mdahlhausen This PR is completed and passed the CI. Could you do a review for its merge? Once approved, we will resolve the conflict.

@lymereJ lymereJ marked this pull request as ready for review August 28, 2024 16:10
@jugonzal07
Copy link
Collaborator Author

@mdahlhausen -- I went ahead and fixed the merge conflicts, they were related to rubocop fixes as far as I could tell and not in any way relevant to the PR updates.

@mdahlhausen
Copy link
Collaborator

@jugonzal07 Looks good to me. Perhaps worth going through and making sure all the conversations are resolved. There is one on renaming the pump name for example that looks unaddressed.

Merge at will.

@weilixu weilixu merged commit d3434a9 into master Sep 4, 2024
1 of 2 checks passed
@mdahlhausen mdahlhausen deleted the primary_secondary_loops branch September 4, 2024 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AppendixG Methods to enable the Appendix G model workflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants