Avail sch missing dx coils cleanup and converted get input to JSON by Nigusse · Pull Request #11012 · NatLabRockies/EnergyPlus

Nigusse · 2025-03-27T16:24:10Z

Pull request overview

New Feature: Adds missing availability schedule input field for elven DX cooling and heating Coils.
Fixes issue Incorrect indices of numeric and alpha fields used for input error reporting in Variable Speed DX Coils #11013
Fixes issue Availability schedule in Coil:Cooling:DX object is not working #11114

Description of the purpose of this PR

Pull Request Author

Reviewer

github-actions · 2025-03-27T17:34:01Z

⚠️ Regressions detected on macos-14 for commit `b8abace`

Regression Summary

BND: 12
MTD: 6
MTR Small Diffs: 3
Table Big Diffs: 2
Table String Diffs: 8
ERR: 6
ESO Small Diffs: 2

github-actions · 2025-03-27T20:11:36Z

⚠️ Regressions detected on macos-14 for commit `0b5e470`

Regression Summary

BND: 12
MTD: 6
MTR Small Diffs: 3
Table Big Diffs: 2
Table String Diffs: 8
ERR: 6
ESO Small Diffs: 2

amirroth

Look at my suggested changes and try to propagate.

amirroth · 2025-03-27T20:04:04Z

src/EnergyPlus/DXCoils.cc

@@ -2979,895 +2979,978 @@ void GetDXCoils(EnergyPlusData &state)

    // Loop over the Pumped DX Water Heater Coils and get & load the data
    CurrentModuleObject = HVAC::cAllCoilTypes(HVAC::CoilDX_HeatPumpWaterHeaterPumped);
-    for (DXHPWaterHeaterCoilNum = 1; DXHPWaterHeaterCoilNum <= state.dataDXCoils->NumDXHeatPumpWaterHeaterPumpedCoils; ++DXHPWaterHeaterCoilNum) {
+    auto &s_ip = state.dataInputProcessing->inputProcessor;


I have a giant coil branch in the hopper and there are going to be a lot of conflicts. Just a matter of which one of us will have to fix them. 😄

amirroth · 2025-03-27T20:06:10Z

src/EnergyPlus/DXCoils.cc

-        thisDXCoil.RatedAirVolFlowRate(1) = Numbers(7);
-        if (thisDXCoil.RatedAirVolFlowRate(1) != Constant::AutoCalculate) {
-            if (thisDXCoil.RatedAirVolFlowRate(1) <= 0.0) {
+            std::string const cRatedHeatingCapFieldName = "Rated Heating Capacity";


This can be a constexpr std::string_view. There is no need to construct a dynamic string object at runtime to represent a constant string.

amirroth · 2025-03-27T20:11:53Z

src/EnergyPlus/DXCoils.cc

+            cFieldName = "Evaporator Fan Power Included in Rated COP";
+            std::string fieldValue = s_ip->getAlphaFieldValue(fields, schemaProps, "evaporator_fan_power_included_in_rated_cop"); // Alphas(2)
+            BooleanSwitch fanPowerIncluded = static_cast<BooleanSwitch>(getYesNoValue(Util::makeUPPER(fieldValue)));
+            switch (fanPowerIncluded) {


A cleaner way to do this. If you are just making a single comparison, if is better/faster than switch. Note the ShowSevereInvalidBool error wrapper.

if (fanPowerIncluded != BooleanSwitch::Invalid) { thisDXCoil.FanPowerIncludedInCOP = static_cast<bool>(fanPowerIncluded); } else { ShowSevereInvalidBool(state, eoh, cFieldName, fieldValue); ErrorsFound = true; }

What makes you believe that fieldValue will be anything other than Yes or No? This just seems to be overhead that's not needed. It's a direct read. No warning message, no gymnastics, just read that input to be true or false.

A2 , \field Evaporator Fan Power Included in Rated COP
\type choice
\key Yes
\key No
\default Yes

So what would that 1 line be? I'll paste together what I see here but am probably wrong. 1 line and done.

thisDXCoil.FanPowerIncludedInCOP = static_cast<bool>(static_cast<BooleanSwitch>(getYesNoValue(Util::makeUPPER(fieldValue))));

This seems to be your corner. That would work. You actually don't need the static_cast<BooleanSwitch>.

amirroth · 2025-03-27T20:13:13Z

src/EnergyPlus/DXCoils.cc

+            if (!whPLFCurveName.empty()) {
+                thisDXCoil.CrankcaseHeaterCapacityCurveIndex = Curve::GetCurveIndex(state, whPLFCurveName);
+                if (thisDXCoil.CrankcaseHeaterCapacityCurveIndex == 0) { // can't find the curve
+                    ShowSevereError(state, format("{} = {}:  {} not found = {}", CurrentModuleObject, thisDXCoil.Name, cFieldName, whPLFCurveName));


There is also an error wrapper for missing objects.

ShowSevereItemNotFound(state, eoh, cFieldName, whPLFCurveName);`

amirroth · 2025-03-27T20:15:32Z

src/EnergyPlus/DXCoils.cc

+            if (!fieldValue.empty()) {
+                thisDXCoil.HCapFAirFlow = GetCurveIndex(state, fieldValue);
+                if (thisDXCoil.HCapFAirFlow == 0) {
+                    ShowSevereError(state, format("{}{}=\"{}\", invalid", RoutineName, CurrentModuleObject, thisDXCoil.Name));


Another ShowSevereItemNotFound. See UtilityRoutines for wrappers for the common errors, e.g., EmptyField, InvalidKey, DuplicateName, BadMin.

amirroth · 2025-03-27T20:24:48Z

src/EnergyPlus/DXCoils.cc

@@ -14988,31 +15071,29 @@ void GetFanIndexForTwoSpeedCoil(
    int BranchNum;
    int CompNum;

+    auto &thisDXCoil = state.dataDXCoils->DXCoil(CoolingCoilIndex);
+
    FoundBranch = 0;
    FoundAirSysNum = 0;
    SupplyFanIndex = 0;
    SupplyFanName = "n/a";
    for (AirSysNum = 1; AirSysNum <= NumPrimaryAirSys; ++AirSysNum) {


While you are at it, may want to downscope the loop induction variables.

amirroth · 2025-03-27T20:25:51Z

src/EnergyPlus/VariableSpeedCoils.cc

@@ -172,20 +172,22 @@ namespace VariableSpeedCoils {
            SpeedCal = SpeedNum;
        }

-        if ((state.dataVariableSpeedCoils->VarSpeedCoil(DXCoilNum).VSCoilType == HVAC::Coil_CoolingWaterToAirHPVSEquationFit) ||
-            (state.dataVariableSpeedCoils->VarSpeedCoil(DXCoilNum).VSCoilType == HVAC::Coil_CoolingAirToAirVariableSpeed)) {
+        auto &varSpeedCoil = state.dataVariableSpeedCoils->VarSpeedCoil(DXCoilNum);


Good lord, the conflicts are going to be massive.

amirroth · 2025-03-27T20:26:54Z

src/EnergyPlus/VariableSpeedCoils.cc

-                        ShowContinueError(state, format("...required {} is blank.", cAlphaFields(AlfaFieldIncre)));
+                std::string fieldName;
+                for (int I = 1; I <= varSpeedCoil.NumOfSpeeds; ++I) {
+                    fieldName = "speed_" + std::to_string(I) + "_reference_unit_gross_rated_total_cooling_capacity";


format instead of string concatenate.

amirroth · 2025-03-27T20:27:55Z

src/EnergyPlus/VariableSpeedCoils.cc

-            }
+                //"defrost_control",
+                cFieldName = "Defrost Control"; // cAlphaFields(8)
+                std::string defrostControl = s_ip->getAlphaFieldValue(fields, schemaProps, "defrost_control");


getEnumValue.

amirroth · 2025-03-27T20:30:32Z

src/EnergyPlus/WaterToAirHeatPump.cc

        // SUBROUTINE LOCAL VARIABLE DECLARATIONS:
        int HPNum; // The Water to Air HP that you are currently loading input into
        int NumCool;
        int NumHeat;


I wish I would have told you to hold off on this until I was done with what I was doing. Too late. Get this merged and I will deal with it. Lesson learned.

Nigusse · 2025-03-27T20:44:04Z

@amirroth Thanks for your feedback comments; I will address them. This can be merged early, as the new feature will be based on the work from this branch.

mitchute · 2025-10-08T15:07:56Z

@Nigusse are you still intending this to be included in this release?

Nigusse · 2025-10-08T15:19:49Z

@mitchute Yes, it has been ready for review. I will resolve the conflicts and push an updated branch by today.

Nigusse · 2025-10-08T17:33:57Z

@Nigusse can you explain these new warnings in the err file? This is for HeatPumpWaterHeater:
+   ************* There are 6 unused objects in input.
+   ************* Use Output:Diagnostics,DisplayUnusedObjects; to see them.

@rraustad I will check this.

github-actions · 2025-10-08T18:28:33Z

⚠️ Regressions detected on ubuntu-24.04 for commit `105387c`

Regression Summary

BND: 12
MTD: 6
MTR Small Diffs: 4
EIO: 1
ESO Small Diffs: 3
Table Small Diffs: 1
ERR: 6
IDF: 2

Nigusse · 2025-10-08T18:32:31Z

Below is the unused objects error message running the example file locally:

   ** Warning ** The following lines are "Unused Objects".  These objects are in the input
   **   ~~~   **  file but are never obtained by the simulation and therefore are NOT used.
   **   ~~~   **  Each unused object is shown.
   **   ~~~   **  See InputOutputReference document for more details.
   ************* Object=Coil:WaterHeating:AirToWaterHeatPump:Pumped=HPWHDXCoil
   **   ~~~   ** Object=Coil:WaterHeating:AirToWaterHeatPump:Pumped=HPWHOutdoorDXCoil
   **   ~~~   ** Object=Coil:WaterHeating:AirToWaterHeatPump:Pumped=HPWHPlantDXCoil
   **   ~~~   ** Object=Coil:WaterHeating:AirToWaterHeatPump:Pumped=HPWHScheduledDXCoil
   **   ~~~   ** Object=Coil:WaterHeating:AirToWaterHeatPump:Pumped=Zone4HPWHDXCoil
   **   ~~~   ** Object=Coil:WaterHeating:AirToWaterHeatPump:Pumped=Zone4bHPWHDXCoil
   ** Warning ** The following schedule names are "Unused Schedules".  These schedules are in the idf
   **   ~~~   **  file but are never obtained by the simulation and therefore are NOT used.
   ************* Schedule:Year or Schedule:Compact or Schedule:File or Schedule:Constant=SSBWATERTEMPSCHEDULE

I am going to remove the unused schedule SSBWATERTEMPSCHEDULE object.

github-actions · 2025-10-08T19:50:36Z

⚠️ Regressions detected on ubuntu-24.04 for commit `8923c57`

Regression Summary

BND: 12
MTD: 6
MTR Small Diffs: 4
EIO: 1
ESO Small Diffs: 3
Table Small Diffs: 1
Audit: 1
ERR: 6
IDF: 2

mitchute · 2025-10-08T20:52:12Z

@Nigusse I have the Mac build errors fixed locally. I can push them up if you'd like.

mitchute · 2025-10-08T21:10:08Z

@Nigusse I went ahead and cleaned up those Mac build issues, and ran unit tests locally. All passed.

github-actions · 2025-10-08T21:56:41Z

⚠️ Regressions detected on ubuntu-24.04 for commit `e9ee899`

Regression Summary

BND: 12
MTD: 6
MTR Small Diffs: 4
EIO: 1
ESO Small Diffs: 3
Table Small Diffs: 1
Audit: 1
ERR: 6
IDF: 2

github-actions · 2025-10-08T22:00:32Z

⚠️ Regressions detected on macos-14 for commit `e9ee899`

Regression Summary

BND: 12
MTD: 6
MTR Small Diffs: 3
Audit: 1
ERR: 6
IDF: 2
ESO Small Diffs: 2

Nigusse · 2025-10-09T13:59:54Z

@mitchute Thanks.

mitchute · 2025-10-09T17:24:05Z

This looks ready. Anything else here?

rraustad · 2025-10-09T18:00:17Z

Doesn't look like this comment was addressed but it's an easy 1-line fix.

Nigusse · 2025-10-09T18:21:21Z

@mitchute @rraustad I will address it shortly.

…Coils

Nigusse · 2025-10-09T18:41:44Z

I have found that the same fatal error message is missing in other DX Coils as well. I am addressing them all.

github-actions · 2025-10-09T20:08:23Z

⚠️ Regressions detected on ubuntu-24.04 for commit `966ff13`

Regression Summary

BND: 12
MTD: 6
MTR Small Diffs: 4
EIO: 1
ESO Small Diffs: 3
Table Small Diffs: 1
Audit: 1
ERR: 6
IDF: 2

rraustad · 2025-10-09T20:39:03Z

@mitchute if this comes back clean then it should be ready to merge. We've pounded on this one for some time now.

mitchute · 2025-10-09T20:45:05Z

@mitchute if this comes back clean then it should be ready to merge. We've pounded on this one for some time now.

Thanks @rraustad @Nigusse and all involved. Hope to merge this soon.

github-actions · 2025-10-09T21:27:19Z

⚠️ Regressions detected on macos-14 for commit `966ff13`

Regression Summary

BND: 12
MTD: 6
MTR Small Diffs: 3
Audit: 1
ERR: 6
IDF: 2
ESO Small Diffs: 2

mitchute · 2025-10-09T21:31:43Z

Some regressions, as expected, but this is clean otherwise. Merging.

Nigusse added 17 commits March 7, 2025 13:40

Variable Speed Coils cleanup; best coding practice

ba33276

WaterToAir HeatPump Simple Cleanup

e30e145

DXCoils module cleanup

69bd519

Initial get Input refactor

f047d99

W2AHP-Simple GetInput to JSON

0d7c9b0

VS Heat and Cool EqnFit Coils GetInput to JSON

36fcb7a

VS Cool Coils GetInput to JSON

49751bf

VS Cool Coils Default Init and cleanup

eeb5122

VS Heat Coil GetInput to JSON

b0f0ac3

VS Water Heat Coil GetInput to JSON

df6947e

Cleanup and applied getXFieldValue functions

dacc13f

Water2AirHP Para Estimation GetInput to JSON

52201c3

Pumped and Wrapped WaterHeater GetInput to JSON

c437958

JSON Get Input Cleanup

0ce1e54

Merged in branch 'develop'; resolved merge conflict

0854e9a

Merge branch 'develop' into avail_sch_missing_DX_coils_cleanup_getInput

a1f87a2

Cleanup; removed unused variables

bb7ad9b

Nigusse added the NewFeature Includes code to add a new feature to EnergyPlus label Mar 27, 2025

Fixed unit tests failed due to get input changes

575c650

amirroth reviewed Mar 27, 2025

View reviewed changes

Nigusse added 7 commits March 29, 2025 13:09

replaced switch with if for single comparison and cleanup

8ce66ed

propagated ShowSevereItemNotFound() function

d08d628

linearized induction variables

0e71c55

applied getEnumValue for defrost control and strategy

421b84d

replaced string concatenation with format

4c4367c

applied clang format

6032037

addressed incomplete change of enum use for defrost control

29488a4

Corrected a Rules Fortran File Name

fbec706

Merged in develop; resolved conflicts

f55d777

Nigusse added 2 commits October 8, 2025 14:37

Removed unused schedule object from HeatPumWaterHeater test file

9452cc9

Removed unused variable warning

7e2cef6

unused vars

95c251c

Fatal error when the avail sch name is specified but not found in DX …

f7f538a

…Coils

Merge branch 'develop' into avail_sch_missing_DX_coils_cleanup_getInput

8aca9ab

mitchute merged commit 2ab91ec into develop Oct 9, 2025
11 checks passed

mitchute deleted the avail_sch_missing_DX_coils_cleanup_getInput branch October 9, 2025 21:32

Nigusse mentioned this pull request Oct 29, 2025

v25.2.0 RC1 incorrect unused object warning #11304

Closed

Conversation

Nigusse commented Mar 27, 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!

github-actions bot commented Mar 27, 2025

⚠️ Regressions detected on macos-14 for commit b8abace

Uh oh!

github-actions bot commented Mar 27, 2025

⚠️ Regressions detected on macos-14 for commit 0b5e470

Uh oh!

amirroth 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!

rraustad Mar 27, 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!

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!

Nigusse commented Mar 27, 2025

Uh oh!

mitchute commented Oct 8, 2025

Uh oh!

Nigusse commented Oct 8, 2025

Uh oh!

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

Uh oh!

Uh oh!

github-actions bot commented Oct 8, 2025

⚠️ Regressions detected on ubuntu-24.04 for commit 105387c

Uh oh!

Nigusse commented Oct 8, 2025

Uh oh!

github-actions bot commented Oct 8, 2025

⚠️ Regressions detected on ubuntu-24.04 for commit 8923c57

Uh oh!

mitchute commented Oct 8, 2025

Uh oh!

mitchute commented Oct 8, 2025

Uh oh!

github-actions bot commented Oct 8, 2025

⚠️ Regressions detected on ubuntu-24.04 for commit e9ee899

Uh oh!

github-actions bot commented Oct 8, 2025

Nigusse commented Mar 27, 2025 •

edited

Loading

⚠️ Regressions detected on macos-14 for commit `b8abace`

⚠️ Regressions detected on macos-14 for commit `0b5e470`

rraustad Mar 27, 2025 •

edited

Loading

Nigusse commented Oct 8, 2025 •

edited

Loading

⚠️ Regressions detected on ubuntu-24.04 for commit `105387c`

⚠️ Regressions detected on ubuntu-24.04 for commit `8923c57`

⚠️ Regressions detected on ubuntu-24.04 for commit `e9ee899`

⚠️ Regressions detected on macos-14 for commit `e9ee899`

rraustad commented Oct 9, 2025 •

edited

Loading

Nigusse commented Oct 9, 2025 •

edited

Loading

⚠️ Regressions detected on ubuntu-24.04 for commit `966ff13`

⚠️ Regressions detected on macos-14 for commit `966ff13`

mitchute commented Oct 9, 2025 •

edited

Loading