-
Notifications
You must be signed in to change notification settings - Fork 418
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
Addressed FanCoil with CyclingFan control using Fan:SystemModel has different fan power than Fan:OnOff #7380
Conversation
Diffs are expected in the example file "HVACTemplate-5ZoneFanCoil-DOAS.idf", because the FanCoil units has Fan:SystemModel fan type and the capacity control method is CyclingFan. |
Yes, diffs are indeed expected and acceptable. Testing locally now. |
@@ -3594,8 +3594,7 @@ namespace FanCoilUnits { | |||
ZoneCompTurnFansOn, | |||
ZoneCompTurnFansOff); | |||
} else { | |||
HVACFan::fanObjs[FanCoil(FanCoilNum).FanIndex]->simulate( | |||
FanCoil(FanCoilNum).LowSpeedRatio, ZoneCompTurnFansOn, ZoneCompTurnFansOff, _); | |||
HVACFan::fanObjs[FanCoil(FanCoilNum).FanIndex]->simulate(_, ZoneCompTurnFansOn, ZoneCompTurnFansOff, _); |
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.
Can you walk me through this change? By not passing a speed ratio, does this force the fan object to operate at a single discrete mode? Is that what fixes the fan to operate just like the Fan:OnOff?
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.
We do not want to pass the fan speed ratio for cycling operating mode. FanCoilUnits cycling mode are designed such that the fan coil unit cycles at each speed level. Thus passing speed ratio will result incorrect fan power calculation. By not passing the fan speed ratio, the fansystem model locally calculates average flow fraction instead, for the fan power calculation.
Fans::SimulateFanComponents( | ||
FanCoil(FanCoilNum).FanName, FirstHVACIteration, FanCoil(FanCoilNum).FanIndex, 0.0, ZoneCompTurnFansOn, ZoneCompTurnFansOff); | ||
} else { | ||
HVACFan::fanObjs[FanCoil(FanCoilNum).FanIndex]->simulate(0.0, ZoneCompTurnFansOn, ZoneCompTurnFansOff, _); |
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.
Is this change required for fixing this defect or is this an unrelated change?
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.
If you referring to the last line where I set 0.0 for speed ratio, is to turn off the fan. Simple because the unit is off if no fan speed is specified.
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.
The fancoil unit is expected to operate at one of the three fan speed levels, if the fan speed is not set yet, then the fan coil unit must be off. Otherwise, what does the fancoil unit do if no fan speed set yet? I observed such a case when I was debugging the defect.
EXPECT_NEAR(FanCoil(1).PLR, 0.753, 0.001); | ||
EXPECT_NEAR(QZnReq, QUnitOut, 5.0); | ||
EXPECT_NEAR(Node(1).MassFlowRate, thisFanCoil.PLR * thisFanCoil.MaxAirMassFlow * thisFanCoil.MedSpeedRatio, 0.0000000001); | ||
} |
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.
I'm not asking you to make a change to the unit test right now but I will say it would be nice to set up a unit test with both fan onoff and system model connected to identical but separate systems. Then you could simulate both of them and check that the values are equal. This would avoid having to hardwire expected values in the unit test. Just a note for future tests like this.
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.
Ok. @Myoldmopar Thanks.
I just ran the OnOff and the SystemModel defect files with both develop and this branch. I see the big difference in results in the develop branch between the two files. With this branch I see the results come together for the two fan types. I think this is ready to go in, but I'd still like a clarification of the changes on my review above so I fully understand it and so they are documented here for the future. Thanks @Nigusse . |
(Also, I just ran unit test suite locally from this branch with develop merged in and it ran fine.) |
Thanks for the clarifications @Nigusse. Merging now. |
Pull request overview
Fixes #7086. Fixed performance differences between a fan coil unit with cycling fan using Fan:OnOff and Fan:SystemMode. Now, fancoil units with cycling fan operating mode, will give the same result using either Fan:OnOff or Fan:SystemMode. Diffs are expected when a FanCoil unit uses Fan:SystemModel fan type and the capacity control method is CyclingFan.
Work Checklist
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
Review Checklist
This will not be exhaustively relevant to every PR.