Add function to associate airloop index to variable speed coil by lymereJ · Pull Request #11243 · NatLabRockies/EnergyPlus

lymereJ · 2025-09-30T20:00:19Z

Pull request overview

Fixes AirflowNetwork validation issue for variable speed coils #11242

Description of the purpose of this PR

Pull Request Author

Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
If changes fix a defect, the fix should be demonstrated in plots and descriptions

Reviewer

…peed coils for AFN validation.

lymereJ

Walk-through:

lymereJ · 2025-09-30T21:08:40Z

src/EnergyPlus/VariableSpeedCoils.cc

        return state.dataVariableSpeedCoils->VarSpeedCoil(DXCoilNum).PartLoadRatio;
    }

+    void SetVarSpeedDXCoilAirLoopNumber(EnergyPlusData &state, std::string const &CoilName, int const AirLoopNum)


New function to associate airloop index to variable speed coil.

lymereJ · 2025-09-30T21:08:59Z

tst/EnergyPlus/unit/AirflowNetworkHVAC.unit.cc

+    state->dataVariableSpeedCoils->GetCoilsInputFlag = false;
+    state->dataVariableSpeedCoils->VarSpeedCoil.allocate(2);
+    state->dataVariableSpeedCoils->VarSpeedCoil(1).Name = "Super Coil";
+    state->dataVariableSpeedCoils->VarSpeedCoil(2).Name = "Super Heating Coil";
+
    state->afn->validate_distribution();
+    compare_err_stream("");


Adapt existing unit test to check for the error stream.

I've confirmed the defect by adding these to develop and testing.

$ make -j 23 && ctest -R AirflowNetwork_ValidateDistCoils --output-on-failure ... [ 76%] Built target TestEnergyPlusCallbacks [100%] Built target energyplus_tests Test project /home/mitchute/Projects/EnergyPlus/build Start 899: EnergyPlusFixture.AirflowNetwork_ValidateDistCoils 1/1 Test #899: EnergyPlusFixture.AirflowNetwork_ValidateDistCoils ...SIGTRAP***Exception: 1.22 sec Note: Google Test filter = EnergyPlusFixture.AirflowNetwork_ValidateDistCoils [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from EnergyPlusFixture [ RUN ] EnergyPlusFixture.AirflowNetwork_ValidateDistCoils /home/mitchute/Projects/EnergyPlus/tst/EnergyPlus/unit/Fixtures/EnergyPlusFixture.cc:219: Failure Expected equality of these values: expected_string Which is: "" stream_str Which is: " ** Severe ** SetDXCoilAirLoopNumber: Could not find Coil \"Name=\"Super Coil\"\n ** Severe ** SetDXCoilAirLoopNumber: Could not find Coil \"Name=\"Super Heating Coil\"\n" With diff: @@ -1,1 +1,2 @@ -"" + ** Severe ** SetDXCoilAirLoopNumber: Could not find Coil \"Name=\"Super Coil\" + ** Severe ** SetDXCoilAirLoopNumber: Could not find Coil \"Name=\"Super Heating Coil\"\n

rraustad · 2025-10-10T23:05:10Z

src/EnergyPlus/AirflowNetwork/src/Solver.cpp

                        ErrorsFound = true;
                    } else {
-                        SetDXCoilAirLoopNumber(m_state, DisSysCompCoilData(i).name, DisSysCompCoilData(i).AirLoopNum);
+                        SetVarSpeedDXCoilAirLoopNumber(m_state, DisSysCompCoilData(i).name, DisSysCompCoilData(i).AirLoopNum);


Well obviously!

rraustad · 2025-10-10T23:06:13Z

src/EnergyPlus/VariableSpeedCoils.cc

+
+        WhichCoil = Util::FindItemInList(CoilName, state.dataVariableSpeedCoils->VarSpeedCoil);
+        if (WhichCoil != 0) {
+            state.dataVariableSpeedCoils->VarSpeedCoil(WhichCoil).AirLoopNum = AirLoopNum;


Where exactly is this used? AirFlowNetwork?

That's correct.

I mean, how does the VS coil use this information? AirflowNetwork already knows the AirLoopNum and is passing that number to the coil model. Why does the coil need to know the AirLoopNum?

I see. The DXCoil model uses that information like this. I don't see either of these variables set in the VS coil model.

state.dataAirLoop->LoopDXCoilRTF = max(thisDXCoil.CoolingCoilRuntimeFraction, thisDXCoil.HeatingCoilRuntimeFraction); if (thisDXCoil.AirLoopNum > 0) { state.dataAirLoop->AirLoopAFNInfo(thisDXCoil.AirLoopNum).AFNLoopDXCoilRTF = max(thisDXCoil.CoolingCoilRuntimeFraction, thisDXCoil.HeatingCoilRuntimeFraction); }

So the VS coil model would also have to do things like this so that AFN has that information.

What is strange with these functions that set the air loop num in a coil model, is that when the coil is simulated the global state.dataSize->CurSysNum is always set when air loops are simulated. So the coil model already knows the AirLoopNum associated with this coil. These functions aren't even needed. Now it may be tricky to internally set up this coil variable based on whether or not AFN is active but I think this could be done as a one-time check in init. Anyway, this is the way it's done now so no reason to go out of scope for this issue. I still think there is more work in the VS coil model before this would work as AFN intends (e.g., the coil model needs the code I show above). I think that's the only other thing needed in the VS coil code.

@rraustad Airloopnum is also used for reporting the airloop name to some of the table reports (but I'm not sure it's consistent across all of the coil types).
And then there's the coil selection report that has it's own tracking of the coil airloopnum and sets this is (nearly?) every ReportCoilSelection::setCoil* function:
c->airloopNum = curSysNum;.

And here's some of the table output for the HeatPumpAuto example file. The first two tables don't know the airloop name, but the next one does?

And then later, we have other reports that show the airloop name

And equivalent tables for heating coils do not even report airloop name.

@rraustad I agree that every coil should simply store it's airloop num from curSysNum at the top of its sizing function. Maybe there's a timing issue for the AFN usage?

@lymereJ Maybe we can fix the "N/A" in the "DX Heating Coils" tables in this PR by adding thisDXCoil.AirLoopNum=state.dataSize->CurSysNum at the top of SizeDXCoil?

@lymereJ @rraustad Oh, but now I see and AirloopNum>0 is being used to tell the DX coil if it's in an AFN loop?!
Then setting AirLoopNum could break things. That should really be a separate flag that gets set from AFN on the coil.

I'll work on a separate PR to fix the table (just use CurSysNum instead of the coil's AirLoopNum). And I'll check other coil types to see if any others are assuming (incorrectly) that AirLoopNum (or equivalent) is set properly for non-AFN runs.

Setting AirloopNum to know if AFN is active is kind of klunky because that data is also used for table reporting. Whatever the AFN variables are then if AFN == Active && AirloopNum > 0 would be better.

Setting AirloopNum to know if AFN is active is kind of klunky because that data is also used for table reporting. Whatever the AFN variables are then if AFN == Active && AirloopNum > 0 would be better.

Can we assume if there's AFN with distribution, then all coils are part of that. Or should the current AFN function calls to the coils be used to set a coil-specific AFN flag?

I am not sure how that is set up but it sounds like your on the right track with thinking AirLoopNum > 0 is the key to knowing when AFN is active and using that coil model.

lymereJ · 2025-10-11T03:14:25Z

src/EnergyPlus/VariableSpeedCoils.cc

+        if (varSpeedCoil.AirLoopNum > 0) {
+            state.dataAirLoop->AirLoopAFNInfo(varSpeedCoil.AirLoopNum).AFNLoopDXCoilRTF = max(varSpeedCoil.RunFracCool, varSpeedCoil.RunFracHeat);
+        }


Adding missing AFNLoopDXCoilRTF assignment for variable speed coils as pointed out by @rraustad, here.

mitchute

The fixes here look reasonable to me. Please comment on whether AirLoopNum should be initialized to some value. I'll defer to @mjwitte and @rraustad on final approval.

src/EnergyPlus/VariableSpeedCoils.hh

mitchute · 2025-10-15T21:53:26Z

tst/EnergyPlus/unit/AirflowNetworkHVAC.unit.cc

+    state->dataVariableSpeedCoils->GetCoilsInputFlag = false;
+    state->dataVariableSpeedCoils->VarSpeedCoil.allocate(2);
+    state->dataVariableSpeedCoils->VarSpeedCoil(1).Name = "Super Coil";
+    state->dataVariableSpeedCoils->VarSpeedCoil(2).Name = "Super Heating Coil";
+
    state->afn->validate_distribution();
+    compare_err_stream("");


I've confirmed the defect by adding these to develop and testing.

$ make -j 23 && ctest -R AirflowNetwork_ValidateDistCoils --output-on-failure ... [ 76%] Built target TestEnergyPlusCallbacks [100%] Built target energyplus_tests Test project /home/mitchute/Projects/EnergyPlus/build Start 899: EnergyPlusFixture.AirflowNetwork_ValidateDistCoils 1/1 Test #899: EnergyPlusFixture.AirflowNetwork_ValidateDistCoils ...SIGTRAP***Exception: 1.22 sec Note: Google Test filter = EnergyPlusFixture.AirflowNetwork_ValidateDistCoils [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from EnergyPlusFixture [ RUN ] EnergyPlusFixture.AirflowNetwork_ValidateDistCoils /home/mitchute/Projects/EnergyPlus/tst/EnergyPlus/unit/Fixtures/EnergyPlusFixture.cc:219: Failure Expected equality of these values: expected_string Which is: "" stream_str Which is: " ** Severe ** SetDXCoilAirLoopNumber: Could not find Coil \"Name=\"Super Coil\"\n ** Severe ** SetDXCoilAirLoopNumber: Could not find Coil \"Name=\"Super Heating Coil\"\n" With diff: @@ -1,1 +1,2 @@ -"" + ** Severe ** SetDXCoilAirLoopNumber: Could not find Coil \"Name=\"Super Coil\" + ** Severe ** SetDXCoilAirLoopNumber: Could not find Coil \"Name=\"Super Heating Coil\"\n

mitchute · 2025-10-15T21:55:58Z

I'll work on a separate PR to fix the table (just use CurSysNum instead of the coil's AirLoopNum). And I'll check other coil types to see if any others are assuming (incorrectly) that AirLoopNum (or equivalent) is set properly for non-AFN runs.

@mjwitte no rush on this, just pulling it out here so we don't lose it.

lymereJ added 2 commits September 30, 2025 12:48

Add new function to get the air loop index associated with variable s…

b0fa285

…peed coils for AFN validation.

Merge branch 'develop' into afn_var_spd_missing_validation

f7b090a

lymereJ added Defect Includes code to repair a defect in EnergyPlus AirflowNetwork Related primarily on airflow-network portions of the codebase labels Sep 30, 2025

Update unit test.

88f492b

lymereJ commented Sep 30, 2025

View reviewed changes

lymereJ added this to the EnergyPlus 25.2 milestone Sep 30, 2025

lymereJ marked this pull request as ready for review September 30, 2025 21:10

rraustad reviewed Oct 10, 2025

View reviewed changes

lymereJ and others added 4 commits October 10, 2025 19:49

Add missing AFN loop DX Coil RTF.

2ba9830

Merge branch 'develop' into afn_var_spd_missing_validation

2a1369f

Max of heating and cooling; Same approach as DXCoils.

08564e0

Don't need these anymore.

a20d3cd

lymereJ commented Oct 11, 2025

View reviewed changes

Only relevant when simulating AFN distribution.

35c3482

mitchute reviewed Oct 15, 2025

View reviewed changes

lymereJ added 2 commits October 23, 2025 09:05

Comments and init.

4562f4b

Merge branch 'develop' into afn_var_spd_missing_validation

2fbb7fc

mitchute merged commit 65ffac0 into develop Oct 24, 2025
11 checks passed

mitchute deleted the afn_var_spd_missing_validation branch October 24, 2025 03:09

Conversation

lymereJ commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request overview

Description of the purpose of this PR

Pull Request Author

Reviewer

Uh oh!

lymereJ left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rraustad Oct 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mitchute left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mitchute commented Oct 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

lymereJ commented Sep 30, 2025 •

edited

Loading

rraustad Oct 11, 2025 •

edited

Loading