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 Ffactor construction with surface adjacency #1834

Merged
merged 1 commit into from
Oct 22, 2024
Merged

Conversation

mdahlhausen
Copy link
Collaborator

@mdahlhausen mdahlhausen commented Oct 21, 2024

Pull request overview

  • Fixes UA calculation failure in test_bldg_13 #1829
  • The error is caused by a surface adopting the construction of the adjacent surface, an FfactorGroundFloor construction, which does not have a U factor, so the surface.uFactor.get call fails.
  • The model is malformed - to surfaces are adjacent, but one is assigned an FfactorGroundFloor construction. That ought to not be a valid construction for surfaces with another surface as the boundary condition.

Fix:

  • Delete the FfactorGroundFloor construction assignment for the two adjacent surfaces; that construction was only used on those two surfaces. Have the adjacent surfaces use the default construction.
  • Add a check to make sure the optional U factor is initialized.

Pull Request Author

  • Method changes or additions
  • 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

  • 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
  • CI status: all green or justified

- Delete the FfactorGroundFloor construction assignment for the two adjacent surfaces; that construction was only used on those two surfaces. Have the adjacent surfaces use the default construction.
- Add a check to make sure the optional U factor is initialized.
@mdahlhausen mdahlhausen requested a review from weilixu October 21, 2024 18:51
@mdahlhausen mdahlhausen merged commit 6b38041 into master Oct 22, 2024
1 of 2 checks passed
@mdahlhausen mdahlhausen deleted the fix/issue1829 branch October 22, 2024 20:28
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.

UA calculation failure in test_bldg_13
1 participant