Adjust mixed air temp #10582

tanaya-mankad · 2024-06-21T19:28:13Z

Pull request overview

Issue Outdoor air mixer does not check for oversaturation before setting Mixed Air Nodes. #10583

In a few different objects' calculate() functions, there are attempts to "correct" the air conditions when an oversaturated (unphysical) state is achieved after some simulation step.
In this branch, we attempt to correct only the earliest and simplest equipment node (Outdoor Air Mixer) that shows unphysical behavior for a new coil model.
(As early-stage equipment (Outdoor Air Mixer) is used in quite a few models, this change propagated failures into a huge number of regression tests.)

…ust-mixed-air-temp

…ergyPlus into adjust-mixed-air-temp

…ust-mixed-air-temp

rraustad · 2024-06-24T14:55:15Z

src/EnergyPlus/MixedAir.cc

@@ -4681,6 +4681,15 @@ void CalcOAMixer(EnergyPlusData &state, int const OAMixerNum)
    // Mixed air temperature is calculated from the mixed air enthalpy and humidity ratio.
    state.dataMixedAir->OAMixer(OAMixerNum).MixTemp =
        Psychrometrics::PsyTdbFnHW(state.dataMixedAir->OAMixer(OAMixerNum).MixEnthalpy, state.dataMixedAir->OAMixer(OAMixerNum).MixHumRat);
+
+    // Check for saturation temperature > dry-bulb temperature and modify temperature at constant enthalpy
+    auto T_sat =


Don't use auto for scalars. And use definition when possible. Here it's Real64 T_sat. This looks correct otherwise. This makes me wonder how many other mixing calcs need this check. Things like setCoolCoilInletTempForZoneEqSizing (and HumRat) in sizing routines, HXs, etc.

Real64 PsyTsatFnHPb(

rraustad · 2024-06-24T14:55:26Z

tst/EnergyPlus/unit/MixedAir.unit.cc

+
+    MixedAir::SimOAMixer(*state, state->dataAirLoop->OutsideAirSys(1).ComponentName(1), state->dataAirLoop->OutsideAirSys(1).ComponentIndex(1));
+
+    auto T_sat = Psychrometrics::PsyTsatFnHPb(*state, state->dataMixedAir->OAMixer(1).MixEnthalpy, state->dataMixedAir->OAMixer(1).MixPressure);


Real64 T_sat

rraustad · 2024-06-24T15:10:02Z

tst/EnergyPlus/unit/PackagedTerminalHeatPump.unit.cc

-    EXPECT_NEAR(31.1803, state->dataLoopNodes->Node(ATMixer1PriInNode).Temp, 0.001);
-    EXPECT_NEAR(0.0015, state->dataLoopNodes->Node(ATMixer1PriInNode).HumRat, 0.001);
+    EXPECT_NEAR(34.2002, state->dataLoopNodes->Node(ATMixer1PriInNode).Temp, 0.001);
+    EXPECT_NEAR(0.0003, state->dataLoopNodes->Node(ATMixer1PriInNode).HumRat, 0.001);


That's a very dry condition and nowhere near the saturation curve. Something looks odd. Mixer return pressure = 0 in this unit test?

state.dataMixedAir->OAMixer(OAMixerNum).MixPressure = (RecircMassFlowRate * RecircPressure + state.dataMixedAir->OAMixer(OAMixerNum).OAMassFlowRate * state.dataMixedAir->OAMixer(OAMixerNum).OAPressure) / state.dataMixedAir->OAMixer(OAMixerNum).MixMassFlowRate;

Yes, it looks like MixPressure has always been 0.0 in this test.

It looks like we are running into yet another example of a unit test that depends on dataEnvrn variables before they are set. The exact node in question had a Pressure value of 0.
I wonder if we can set a trap for all the tests that might be running into this scenario - it's the 5th one I've seen personally.

I have also seen many. It's nice to know that moving state->dataEnvrn->OutBaroPress = 101325.0; earlier solves the problem.

…ust-mixed-air-temp

Myoldmopar

Looks pretty good to me!

Myoldmopar · 2024-06-25T17:09:51Z

src/EnergyPlus/MixedAir.cc

+        state.dataMixedAir->OAMixer(OAMixerNum).MixTemp = T_sat;
+        state.dataMixedAir->OAMixer(OAMixerNum).MixHumRat =
+            Psychrometrics::PsyWFnTdbH(state, T_sat, state.dataMixedAir->OAMixer(OAMixerNum).MixEnthalpy);
+    }


Pretty minimal fix here! My only suggestion would have been to auto & mixer = state.dataMixedAir->OAMixer(OAMixerNum); so that the code would be reduced to:

Real64 T_sat = Psychrometrics::PsyTsatFnHPb(state, mixer.MixEnthalpy, mixer.MixPressure); if (mixer.MixTemp < T_sat) { mixer.MixTemp = T_sat; mixer.MixHumRat = Psychrometrics::PsyWFnTdbH(state, T_sat, mixer.MixEnthalpy); }

which is much more satisfying :) But not necessary. So just a note.

Myoldmopar · 2024-06-25T17:10:43Z

tst/EnergyPlus/unit/PackagedTerminalHeatPump.unit.cc

@@ -4027,7 +4028,7 @@ TEST_F(EnergyPlusFixture, PTACDrawAirfromReturnNodeAndPlenum_Test)
    // same temperature test as above commented out test (23.15327704750551), now shows 21.2 C
    // how do you mix 2 air streams with T1in=31.18 and T2in=23.15 and get Tout=21.23 ??
    // must be a node enthalpy issue with this unit test?
-    EXPECT_NEAR(21.2316, state->dataLoopNodes->Node(ATMixer1AirOutNode).Temp, 0.001);
+    EXPECT_NEAR(21.2317, state->dataLoopNodes->Node(ATMixer1AirOutNode).Temp, 0.001);


Personal preference, just loosen the tolerance a little. But it's fine.

rraustad

This looks ready to merge.

Myoldmopar · 2024-06-25T17:27:35Z

Do we need some more time spent on the regression failures @rraustad ? It's got 9 files with big ESO diffs, and like 45 with small ESO diffs. I realize the change is obvious, so I'm not demanding it, but wondering your thoughts.

rraustad · 2024-06-25T17:33:34Z

I didn't even look, I was thinking diffs were expected. It's probably a good idea to verify big diffs in at least 1 file so something we did not think of does not slip through.

tanaya-mankad · 2024-06-25T17:41:30Z

I can attempt to trace the diffs in one of the big-diff files - will ask for help if I need it.
@Myoldmopar I'll add your suggested changes as well. What tolerances make sense for the temperatures? (I have no intuition myself in this.)

Myoldmopar · 2024-06-25T17:44:18Z

Thanks @tanaya-mankad !

FWIW, I did re-run regressions on this branch just now but reported time series as monthly. It reduced the number of big diff files from 9 down to 5, but still lots and lots of diffs. I agree it would be worth plotting some results and discussing the diffs. (It's not even necessarily about proving the code change at this point, it's about the after effects. When users or interfaces encounter big diffs with 24.2, we need a place to point them to explain what happened.)

A few plots showing the changes with new behavior will be wonderful. As for the tolerances, the 0.001 was arbitrary anyway. But I don't think we should have unit tests fail because a temperature changes by a thousandth of a degree or whatever. If the tolerance is 0.001 now, but it passes with a tolerance of 0.005, just go with that. Also...if that becomes annoying or weird, you are welcome to leave it.

tanaya-mankad · 2024-07-12T15:28:15Z

Diagram 1 focuses on a few timesteps surrounding the greatest absolute regression diff for UnitarySystem_MultiSpeedDX_EconoStaging.idf. During the time the heating coil is in operation, we choose a timestep with high relative indoor humidity to illustrate changes in the outdoor air mixer. Dry, cold air from outdoors mixing with warm, humid indoor return air causes condensation, rather than a theoretical (unphysical) oversaturated air mix, as would be indicated by the red dotted line in Diagram 2. The light blue constant-temperature line indicates the mixed air conditions described in Issue #10583, which we adjust in this branch to the dark blue line that lies at T_saturation. The physical effect will be that condensate is removed from the air stream, which now is saturated at 100% RH and a higher dry-bulb temperature than previously assumed (the temperature trend seen in Diagram 1).

rraustad · 2024-07-12T15:38:25Z

@tanaya-mankad that is a very good figure that describes the change in results and the psychrometric conditions that accompany this change. The "condensation removed" comment on the psyc chart makes me wonder if moisture really does condense in a mixing box or if there is an "equalization" of air properties when mixing 2 air streams.

nealkruis · 2024-07-12T18:25:52Z

@rraustad in reality, I suspect it depends on the temperature of the ductwork. If the moisture doesn't condense on a surface it probably stays in the air as fog and gets moved along until it settles on something (e.g., a filter) or re-evaporates into the air as a process warms it up. The original problem was that this "correction" was previously happening downstream at some cooling coils regardless of whether the coil was running or not. We could maybe add another issue to decide if we should be handling this phenomenon differently. Would that work for now?

rraustad · 2024-07-12T18:36:33Z

I'm not asking for any other changes here. I was just wondering about the science behind the code correction. Specifically, the act of mixing 2 air streams.

Myoldmopar · 2024-07-15T16:21:38Z

@Myoldmopar I'll add your suggested changes as well. What tolerances make sense for the temperatures? (I have no intuition myself in this.)

@tanaya-mankad did you have any changes to push here? My requests are not critical, and I think this can merge as-is. But if you had local changes for the reference usage and tolerance change, I will wait for those.

tanaya-mankad · 2024-07-15T16:36:40Z

@Myoldmopar, it's ready to merge from my side! I took your advice about trying a halved tolerance and it passed, but I wasn't sure what you meant by "go with that" where that = 0.001, or that = 0.0005. :)

Myoldmopar · 2024-07-15T17:06:39Z

@tanaya-mankad OK, I got a little carried away. I updated your changes to use a reference, but then realized, it would only be a couple more minutes to make them into full methods on the mixer class, so why not? After doing that, I cleaned up the two requests in the unit test code, and pushed. Assuming CI is happy, I'll merge. And if CI is unhappy, I can revert my tweaks and then merge :)

tanaya-mankad · 2024-07-15T17:10:58Z

@Myoldmopar ah, shoot, I thought I had already merged something to fix that! Sorry you had to do it. At this stage, if you're happy, I will stop work on the branch (I think I got carried away with the regressions and spaced on the style issues.)

Myoldmopar · 2024-07-15T17:11:57Z

No problemo, and yeah, I think go ahead and stop here. I am pretty confident CI will be happy and this will merge this afternoon.

Myoldmopar · 2024-07-25T21:05:17Z

This one is still happy as of today, so merging this now. Thanks @tanaya-mankad !

tanaya-mankad added 5 commits April 16, 2024 11:02

Adjust dry-bulb temperature of mixed air if the air state is unphysical.

4f79ab3

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into adj…

f5fe049

…ust-mixed-air-temp

Merge branch 'initialize-node-pressure' of https://github.com/NREL/En…

6a9baef

…ergyPlus into adjust-mixed-air-temp

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into adj…

ddcffb4

…ust-mixed-air-temp

Change expected values for OA node in test.

ee3e6d9

tanaya-mankad added the Defect Includes code to repair a defect in EnergyPlus label Jun 21, 2024

tanaya-mankad self-assigned this Jun 21, 2024

Fix formatting.

0c5ec52

tanaya-mankad requested review from nealkruis and rraustad and removed request for nealkruis June 21, 2024 19:55

tanaya-mankad marked this pull request as ready for review June 24, 2024 14:36

rraustad reviewed Jun 24, 2024

View reviewed changes

tanaya-mankad added 2 commits June 24, 2024 15:50

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into adj…

6ca9c26

…ust-mixed-air-temp

Move dataEnvrn settings above any functions that call SetOANodeValues.

08348b5

tanaya-mankad requested a review from rraustad June 25, 2024 14:06

Myoldmopar approved these changes Jun 25, 2024

View reviewed changes

rraustad approved these changes Jun 25, 2024

View reviewed changes

Merge remote-tracking branch 'origin/develop' into adjust-mixed-air-temp

4d28f23

tanaya-mankad requested review from Myoldmopar and rraustad July 12, 2024 15:31

Myoldmopar added 3 commits July 15, 2024 11:41

Merge remote-tracking branch 'origin/develop' into adjust-mixed-air-temp

750f331

Refactor some mixer code into methods

a9572ac

Loosen tolerance on temperature in unit test

472e3e9

Clang format

abed1f0

Myoldmopar merged commit ef02672 into develop Jul 25, 2024
15 checks passed

Myoldmopar deleted the adjust-mixed-air-temp branch July 25, 2024 21:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adjust mixed air temp #10582

Adjust mixed air temp #10582

tanaya-mankad commented Jun 21, 2024 •

edited

Loading

rraustad Jun 24, 2024

rraustad Jun 24, 2024 •

edited

Loading

rraustad Jun 24, 2024

tanaya-mankad Jun 24, 2024

tanaya-mankad Jun 24, 2024

rraustad Jun 25, 2024

Myoldmopar left a comment

Myoldmopar Jun 25, 2024

Myoldmopar Jun 25, 2024

rraustad left a comment

Myoldmopar commented Jun 25, 2024

rraustad commented Jun 25, 2024

tanaya-mankad commented Jun 25, 2024

Myoldmopar commented Jun 25, 2024

tanaya-mankad commented Jul 12, 2024

rraustad commented Jul 12, 2024

nealkruis commented Jul 12, 2024 •

edited

Loading

rraustad commented Jul 12, 2024

Myoldmopar commented Jul 15, 2024

tanaya-mankad commented Jul 15, 2024

Myoldmopar commented Jul 15, 2024

tanaya-mankad commented Jul 15, 2024

Myoldmopar commented Jul 15, 2024

Myoldmopar commented Jul 25, 2024


		MixedAir::SimOAMixer(*state, state->dataAirLoop->OutsideAirSys(1).ComponentName(1), state->dataAirLoop->OutsideAirSys(1).ComponentIndex(1));

		auto T_sat = Psychrometrics::PsyTsatFnHPb(*state, state->dataMixedAir->OAMixer(1).MixEnthalpy, state->dataMixedAir->OAMixer(1).MixPressure);

Adjust mixed air temp #10582

Adjust mixed air temp #10582

Conversation

tanaya-mankad commented Jun 21, 2024 • edited Loading

Pull request overview

rraustad Jun 24, 2024

Choose a reason for hiding this comment

rraustad Jun 24, 2024 • edited Loading

Choose a reason for hiding this comment

rraustad Jun 24, 2024

Choose a reason for hiding this comment

tanaya-mankad Jun 24, 2024

Choose a reason for hiding this comment

tanaya-mankad Jun 24, 2024

Choose a reason for hiding this comment

rraustad Jun 25, 2024

Choose a reason for hiding this comment

Myoldmopar left a comment

Choose a reason for hiding this comment

Myoldmopar Jun 25, 2024

Choose a reason for hiding this comment

Myoldmopar Jun 25, 2024

Choose a reason for hiding this comment

rraustad left a comment

Choose a reason for hiding this comment

Myoldmopar commented Jun 25, 2024

rraustad commented Jun 25, 2024

tanaya-mankad commented Jun 25, 2024

Myoldmopar commented Jun 25, 2024

tanaya-mankad commented Jul 12, 2024

rraustad commented Jul 12, 2024

nealkruis commented Jul 12, 2024 • edited Loading

rraustad commented Jul 12, 2024

Myoldmopar commented Jul 15, 2024

tanaya-mankad commented Jul 15, 2024

Myoldmopar commented Jul 15, 2024

tanaya-mankad commented Jul 15, 2024

Myoldmopar commented Jul 15, 2024

Myoldmopar commented Jul 25, 2024

tanaya-mankad commented Jun 21, 2024 •

edited

Loading

rraustad Jun 24, 2024 •

edited

Loading

nealkruis commented Jul 12, 2024 •

edited

Loading