-
Notifications
You must be signed in to change notification settings - Fork 389
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
Comment and cleanup of heat balance arrays #9097
Conversation
@mjwitte This branch is ready for review. It's no more than a few naming convention cleanups and a few local variable refactoring in calculating interior solar distribution. Would you help take a look? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xuanluo0113 This looks good. Just a minor comment below.
Also, I don't see any CI results posted on the PR, so I'll check those after the next push.
src/EnergyPlus/DataHeatBalance.hh
Outdated
// Pointers to Surface Data Structure | ||
// AllSurfF AllSurfL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very helpful, but I would pay close attention to the alignment and possibly add |
characters to draw vertical lines to show the groupings.
Also, some of these will equal each other, e.g. HTSurfaceFirst = OpaqOrIntMassSurfaceFirst
.
And comments explaining how these are set if a particular class of surfaces are not present in the zone. e.g. if there are no domes, what are TDDDomeFirst and TDDDomeLast
set to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mjwitte Thanks for the comments! I've committed the changes to make the comments clearer. For TDDDomes, same as Windows, if there are no domes, TDDDomeFirst
is set as 0 and TDDDomeLast
as -1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xuanluo113 Thanks. Much better. Will wait for CI green lights, then merge.
Comment and cleanup of heat balance arrays
Pull request overview
This branch includes a few naming convention cleanups and a few local variable refactoring in calculating interior solar distribution.
Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
Reviewer
This will not be exhaustively relevant to every PR.