-
Notifications
You must be signed in to change notification settings - Fork 440
Fan:SystemModel with discrete speed flow fractions crashes when speeds >= 2 #10846
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
Conversation
for (int i = 1; i <= vrfTU.NumOfSpeedCooling; ++i) { | ||
vrfTU.CoolMassFlowRate[i] = fanSystem->massFlowAtSpeed[i - 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.
Crashes in these places in VRF GetInput because massFlowAtSpeed is allocated in the fan's set_size function and that has not been called yet.
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.
👍
} | ||
} | ||
} | ||
} |
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.
So moved these initializations to Init where the code waits for the fan to size before setting up air flow variables.
EXPECT_NEAR(state->dataHVACVarRefFlow->VRFTU[0].CycRatio, 0.713204045, 0.00001); // was 0.713222413 | ||
} | ||
|
||
TEST_F(EnergyPlusFixture, VRF_MultispeedFan_Test_HardSized) |
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.
New unit test with hard-sized inputs for airflow, capacity and SHR that are duplicate of results from above unit test. These answers are a little different then when using autosize.
|
A clue to the diffs on Mac CI for VRFMultispeedFan. TU2 fan is the only multispeed fan of the 5 TUs in this example file. No Cooling SA Flow Rate changed while No Heating SA Flow Rate did not. @Myoldmopar why is Mac CI picking up diffs while the other CIs do not?
|
Mac is the only one running regressions now. |
@@ -7728,7 +7741,7 @@ void SizeVRF(EnergyPlusData &state, int const VRFTUNum) | |||
} else { | |||
auto *fanSystem = dynamic_cast<Fans::FanSystem *>(state.dataFans->fans(state.dataHVACVarRefFlow->VRFTU(VRFTUNum).FanIndex)); | |||
assert(fanSystem != nullptr); | |||
if (state.dataHVACVarRefFlow->VRFTU(VRFTUNum).CoolMassFlowRate[i] == 0.0) { | |||
if (state.dataHVACVarRefFlow->VRFTU(VRFTUNum).CoolMassFlowRate[i] == 0.0 && !fanSystem->massFlowAtSpeed.empty()) { |
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.
Just in case the fan has not yet been sized.
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.
Interesting. Is that something that actually could happen for a normal IDF run? Or is this just to proof it against missing inits in unit tests?
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 was a normal idf run. The massFlowAtSpeed array is not allocated until Fan::set_size is called. Which may not have been called yet at this point in VRF sizing.
@rraustad it has been 15 days since this pull request was last updated. |
@rraustad it has been 7 days since this pull request was last updated. |
1 similar comment
@rraustad it has been 7 days since this pull request was last updated. |
…45-VRF-crash-with-discrete-system-fan
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 fine as is. I'm curious about the empty checks, but they aren't causing trouble. I'll give this a quick check.
@@ -7728,7 +7741,7 @@ void SizeVRF(EnergyPlusData &state, int const VRFTUNum) | |||
} else { | |||
auto *fanSystem = dynamic_cast<Fans::FanSystem *>(state.dataFans->fans(state.dataHVACVarRefFlow->VRFTU(VRFTUNum).FanIndex)); | |||
assert(fanSystem != nullptr); | |||
if (state.dataHVACVarRefFlow->VRFTU(VRFTUNum).CoolMassFlowRate[i] == 0.0) { | |||
if (state.dataHVACVarRefFlow->VRFTU(VRFTUNum).CoolMassFlowRate[i] == 0.0 && !fanSystem->massFlowAtSpeed.empty()) { |
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.
Interesting. Is that something that actually could happen for a normal IDF run? Or is this just to proof it against missing inits in unit tests?
for (int i = 1; i <= vrfTU.NumOfSpeedCooling; ++i) { | ||
vrfTU.CoolMassFlowRate[i] = fanSystem->massFlowAtSpeed[i - 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.
👍
Good to go, thanks @rraustad ! |
Pull request overview
NOTE: ENHANCEMENTS MUST FOLLOW A SUBMISSION PROCESS INCLUDING A FEATURE PROPOSAL AND DESIGN DOCUMENT PRIOR TO SUBMITTING CODE
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.