Address HSPF2 calculation problems for multispeed DX system #10618

Nigusse · 2024-07-22T14:26:42Z

Pull request overview

Fixes HSPF2 calculation problems for multispeed DX systems #9909
Corrects the HSPF2 calculation procedure for multi-speed DX heating coils to match the 2023 AHRI standards 210/240
HSPF2 value diffs are expected
Fix StandardsRatings: calculation of metrics can crash if certain preconditions aren't met #10651

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.

Nigusse

Code change walkthrough:

the code change now match the 2023 AHRI Standard

Nigusse · 2024-07-22T15:02:28Z

src/EnergyPlus/StandardRatings.cc

+                        // Equation 11.199 AHRI-2023
+                        q_full = q_H3_full + (q_H1_full - q_H3_full) * t_ratio;
+                        // Equation 11.200 AHRI-2023
+                        p_full = p_H3_full + (p_H1_full - p_H3_full) * t_ratio;
                    }


The current code for Case III looks incorrect based on the review of the AHRI 2023 (2020) Standard for Performance Rating of Unitary Air-conditioning & Air-source Heat Pump Equipment. This code change matches the AHRI standard and corrects the logic flaws.

Thanks @Nigusse

Nigusse · 2024-07-22T15:03:21Z

tst/EnergyPlus/unit/DXCoils.unit.cc

@@ -4656,7 +4656,7 @@ TEST_F(EnergyPlusFixture, TestMultiSpeedCoilsAutoSizingOutput)
 ! <DX Heating Coil Standard Rating Information>, Component Type, Component Name, High Temperature Heating (net) Rating Capacity {W}, Low Temperature Heating (net) Rating Capacity {W}, HSPF {Btu/W-h}, Region Number
 DX Heating Coil Standard Rating Information, Coil:Heating:DX:MultiSpeed, ASHP HTG COIL, 34415.4, 20666.4, 6.56, 4
 ! <DX Heating Coil AHRI 2023 Standard Rating Information>, Component Type, Component Name, High Temperature Heating (net) Rating Capacity {W}, Low Temperature Heating (net) Rating Capacity {W}, HSPF2 {Btu/W-h}, Region Number
- DX Heating Coil AHRI 2023 Standard Rating Information, Coil:Heating:DX:MultiSpeed, ASHP HTG COIL, 34697.2, 20948.1, 5.34, 4
+ DX Heating Coil AHRI 2023 Standard Rating Information, Coil:Heating:DX:MultiSpeed, ASHP HTG COIL, 34697.2, 20948.1, 5.35, 4


Modified a failed unit test.

Nigusse

Code change walkthrough for additional issues identified while working on the original defect.

Nigusse · 2024-07-24T17:51:24Z

src/EnergyPlus/StandardRatings.cc

+                Real64 q_H1_full = Q_H1_Full(spnum + 1);
+                Real64 q_H2_full = Q_H2_Full(spnum + 1);
+                Real64 q_H3_full = Q_H3_Full(spnum + 1);
+                Real64 q_H4_full = Q_H4_Full(spnum + 1);


The full heating capacity must be evaluated at the next available speed, not at the current speed. It should be different from the low speed.

It makes sense, and I believe you, but it would be great to get a second opinion on it. Anyone have thoughts on this?

I don't know the standard but what will these values be used for? i.e., these would be used in some equation, so show the equation to support this change. For example, q_H1_full looks to be the capacity at speed 1 so why would we use speed 2 capacity?

These capacities (H1, 2, 3, 4) are used to estimate the full capacity at a specified outdoor a condition through interpolation. The full capacity is then used to bracket the building load to get a speed level of the machine that matches the building load.

Nigusse · 2024-07-24T17:52:03Z

src/EnergyPlus/StandardRatings.cc

+                Real64 p_H1_full = P_H1_Full(spnum + 1);
+                Real64 p_H2_full = P_H2_Full(spnum + 1);
+                Real64 p_H3_full = P_H3_Full(spnum + 1);
+                Real64 p_H4_full = P_H4_Full(spnum + 1);


The full electric power input must be evaluated at the next available speed, not at the current speed. It should be different from the low speed.

Nigusse · 2024-07-24T17:53:24Z

src/EnergyPlus/StandardRatings.cc

                // Equation 11.177 AHRI-2023
                //?? (replaced 62 with 35) in Ratio expression // (t=>35-47/62-47)
                Real64 q_35_low = // q_H1_low + (q_H0_low - q_H1_low) * ((t - (8.33)) / (1.66 - (8.33)));
                    q_H1_low + (q_H0_low - q_H1_low) * ((1.67 - (8.33)) / (16.67 - (8.33)));

                // Equation 11.191 AHRI-2023
-                Real64 N_Hq = min(1.0, (q_H2_int - q_35_low) / (q_H2_full - q_35_low));
+                Real64 N_Hq = (q_H2_full != q_35_low) ? min(1.0, (q_H2_int - q_35_low) / (q_H2_full - q_35_low)) : 0.0;


Modified to allow evaluation of the HSPF2 with flat heating capacity modifier curves function of temperature.

This fixes #10651

FWIW, and this looks like undefined behavior, but I was finding that in a release build with no floating point exceptions enabled, when you had the same Cap FT / Fflow curves, N_Hq was 0 but N_HE was 1.

Nigusse · 2024-07-24T17:54:47Z

src/EnergyPlus/StandardRatings.cc

                // Equation 11.178 AHRI - 2023
                //?? (replaced 62 with 35) in Ratio expression (t=>35 F-47/35-47)
                Real64 p_35_low = // p_H1_low + (p_H0_low - p_H1_low) * ((t - (8.33)) / (1.66 - (8.33)));
                    p_H1_low + (p_H0_low - p_H1_low) * ((1.67 - (8.33)) / (16.67 - (8.33)));

                // Equation 11.194 AHRI-2023
-                Real64 N_HE = min(1.0, (p_H2_int - p_35_low) / (p_H2_full - p_35_low));
+                Real64 N_HE = (p_H2_int != p_35_low) ? min(1.0, (p_H2_int - p_35_low) / (p_H2_full - p_35_low)) : 0.0;


It was modified to allow the evaluation of the HSPF2 with flat heating capacity modifier curves function of temperature.

Nigusse · 2024-07-24T17:58:05Z

src/EnergyPlus/StandardRatings.cc

+                    // equation 11.181
+                    q_low = Q_H2_Int(spnum);
+                    // equation 11.184
+                    p_low = P_H2_Int(spnum);


It was modified the equations used to calculate delivered heat and energy input at the Low Speed to match the 2023 AHRI Standard 210/240.

Nigusse · 2024-07-24T17:58:41Z

src/EnergyPlus/StandardRatings.cc

+                    // equation 11.199
+                    q_hs = Q_H3_Full(spnum + 1) + ((Q_H1_Full(spnum + 1) - Q_H3_Full(spnum + 1)) * ratio);
+                    // equation 11.200
+                    p_hs = P_H3_Full(spnum + 1) + ((P_H1_Full(spnum + 1) - P_H3_Full(spnum + 1)) * ratio);
                }


Modified the equations used to calculate delivered heat and energy input at the High Speed to match the 2023 AHRI Standard 210/240.

OK, so here I see the equations. I still don't know if Q_H1_Full should be the "full" capacity at the speed being evaluated or the full capacity at the next highest speed. What in the standard prompted this change?

My understanding is that the next higher speed level should be used to get the full speed capacity that bounds the current building load.

But if PLR at speed x is < 1 then the full capacity at that same speed would bound the current building load. So where in the standard does it say to use the next highest speed level?

In the standard, different sections may apply to this code: one for a two-speed (two-capacity) machine and another one for a variable-speed machine. They are not an exact match for a discrete-speed machine with more than the two-speed. The HSPF2 calculation is based on the variable speed section of the standard, and it requires some interpretation to fit the discrete multispeed DX heating coil model we have in EnergyPlus. It requires some judgment on how to account for the difference between the EnergyPlus model and the variable speed machine procedure in the AHRI standard. Simply, it needs interpretation of the procedure for implementation in EnergyPlus.

In a variable-speed machine, you have just one full speed, like the two-speed DX cooling coil model in EnergyPlus. In the multispeed DX heating coil, your full speed is a moving target, and depending on your load, it is the next available higher-speed capacity. Let's say a given load is bound between two-speed levels; the full load should point to the higher speed level, not the lower speed. This is my interpretation of how it should be. The current full capacity assignment was to the current speed, so I changed it to the next higher-speed capacity.

Nigusse · 2024-07-24T18:02:14Z

tst/EnergyPlus/unit/DXCoils.unit.cc

@@ -4656,7 +4656,7 @@ TEST_F(EnergyPlusFixture, TestMultiSpeedCoilsAutoSizingOutput)
 ! <DX Heating Coil Standard Rating Information>, Component Type, Component Name, High Temperature Heating (net) Rating Capacity {W}, Low Temperature Heating (net) Rating Capacity {W}, HSPF {Btu/W-h}, Region Number
 DX Heating Coil Standard Rating Information, Coil:Heating:DX:MultiSpeed, ASHP HTG COIL, 34415.4, 20666.4, 6.56, 4
 ! <DX Heating Coil AHRI 2023 Standard Rating Information>, Component Type, Component Name, High Temperature Heating (net) Rating Capacity {W}, Low Temperature Heating (net) Rating Capacity {W}, HSPF2 {Btu/W-h}, Region Number
- DX Heating Coil AHRI 2023 Standard Rating Information, Coil:Heating:DX:MultiSpeed, ASHP HTG COIL, 34697.2, 20948.1, 5.34, 4
+ DX Heating Coil AHRI 2023 Standard Rating Information, Coil:Heating:DX:MultiSpeed, ASHP HTG COIL, 34697.2, 20948.1, 5.76, 4


Modified the HSPF2 values unit test to match latest results. Now the difference between HSPF and HSPF2 reduced to 12.2% from 18.6%.

Nigusse · 2024-07-24T18:05:30Z

tst/EnergyPlus/unit/DXCoils.unit.cc

+
+    EXPECT_TRUE(compare_eio_stream(htg_coil_eio_output, true));
+}
+
 } // namespace EnergyPlus


Added three unit tests. These unit tests evaluate HSPF values for various compressor operation control minimum outdoor air temperature and fan power inputs. The unit tests demonstrate that the HSPF2 values are now closer to the HSPF values.

Nigusse · 2024-07-25T12:37:27Z

See below comparison of the HSPF2 values of develop and this branch:

Develop: percent difference between HSPF and HSPF2 is about 19.0%.

Branch: percent difference between HSPF and HSPF2 is about 8.9%.

Nigusse · 2024-07-25T13:21:42Z

See below comparison of the HSPF2 calculation intermediate outputs for develop and this branch:

Develop:

Branch:

The delivered heating rate is 4979.8W, the same for development and this branch. However, the total electric power input (compressor electric power + electric resistance heater power) decreased to 2389.0W from 2690.4W, resulting in an increased HSPF2 value. Note that the electric resistance heater power input has decreased more than the compressor electric power input increase.

Nigusse · 2024-07-25T13:30:36Z

Attached is a test file used to generate sample results demonstrating the fix.

Issue9909_in.idf.txt

Nigusse · 2024-07-25T15:44:21Z

The regression test results show that 18 example files have diffs in the HSPF2 values. All these 18 example files have Multispeed DX Heating Coils. We found only six unique HSPF2 values since the inputs are similar in multiple example files. Below is a summary of the results based on the six files. The absolute diffs range 0.07 – 0.4 Btu/W-h, and the percent diffs range 1.0% - 8.1%. All the 18 example files with Multispeed DX heating coils in the branch showed increased HSPF2 values. However, they are less than the HSPF values. These results are consistent with the expectations.

Nigusse · 2024-07-25T15:49:30Z

This branch is ready for review.

Myoldmopar

This is fine to me, and I know you've done your due diligence in these changes. I'd like another review opinion on the changes to speed lookups and such, but if not, it's OK.

Myoldmopar · 2024-07-25T21:13:51Z

src/EnergyPlus/StandardRatings.cc

+                Real64 q_H1_full = Q_H1_Full(spnum + 1);
+                Real64 q_H2_full = Q_H2_Full(spnum + 1);
+                Real64 q_H3_full = Q_H3_Full(spnum + 1);
+                Real64 q_H4_full = Q_H4_Full(spnum + 1);


It makes sense, and I believe you, but it would be great to get a second opinion on it. Anyone have thoughts on this?

Myoldmopar · 2024-07-25T21:14:23Z

src/EnergyPlus/StandardRatings.cc

+                        // Equation 11.199 AHRI-2023
+                        q_full = q_H3_full + (q_H1_full - q_H3_full) * t_ratio;
+                        // Equation 11.200 AHRI-2023
+                        p_full = p_H3_full + (p_H1_full - p_H3_full) * t_ratio;
                    }


Thanks @Nigusse

…ed_DX_Htg_Coil_HSPF2

Myoldmopar · 2024-08-16T14:47:05Z

@Nigusse I was going to merge this in, but during local testing I found 3 tests failing. I spent about 30 minutes trying to diagnose the issue, looking at other changes made recently that might interact, but couldn't solve it quick enough. While doing that I also found like 20+ variables in SizeDXCoil that were simply unused. (Assigned but never accessed) So I removed them just because.

If these 3 tests still fail, and you are able to look at them, that would be great. If not, I'll have to do some further testing to figure out what I need to do to the unit tests to get them to work.

Extra info:

This is the obvious culprit, but I couldn't tell quick enough what was needed on the HSPF tests.
If I need to debug further, I'm likely going to roll back to the unmerged version of this branch to get it to pass, then more carefully bring in commits from develop to see exactly what is triggering the failures.

Anyway, I'm going to move on to other PRs for now.

Nigusse · 2024-08-16T17:02:20Z

@Myoldmopar I will update the branch. If the failed unit tests are the new unit tests added, then I know what the problem is. I will look at it later today.

Nigusse · 2024-08-16T18:49:56Z

@Myoldmopar The failed unit test must have impacted by changes somewhere else. Now, the heating capacity is sized to zero. Maybe these unit tests need to be modified.

This commit includes the fix to the problem for both condensers operating even when the operation scheme said that one of them should not be operating. It fixes this for all of the cooling tower models that were not set up to look at this. Results now should that the towers are operating when they are supposed to.

Nigusse · 2024-08-30T12:05:01Z

@Myoldmopar Resolved the conflict and the 18 example files diffs are expected per the original fix.

…multispeed_DX_Htg_Coil_HSPF2

mjwitte · 2024-09-07T12:45:13Z

src/EnergyPlus/DXCoils.cc

@@ -7549,22 +7487,19 @@ void SizeDXCoil(EnergyPlusData &state, int const DXCoilNum)
            PrintFlag = true;
            state.dataSize->DataTotCapCurveIndex = thisDXCoil.CCapFTemp(Mode);
            if (thisDXCoil.DXCoilType_Num == HVAC::CoilDX_CoolingTwoStageWHumControl) {
-                SizingMethod = HVAC::CoolingCapacitySizing;


@Nigusse @rraustad Are these changes to DXCoils.cc supposed to be in this PR?

It looks like cleaning up unused information. I guess this can be done here as long as diffs show only for HSPF calculations.

No. I did not make this change.

@Nigusse @rraustad Are these changes to DXCoils.cc supposed to be in this PR?

I think, @Myoldmopar might have made sizing related cleanup.

Ah, ok, I see that commit.

github-actions · 2024-09-07T13:24:19Z

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

Regression Summary

EIO: 16
Table Big Diffs: 18

Myoldmopar · 2024-09-09T20:53:19Z

No conflicts here 🎉 , but I'll still pull develop in locally to check nothing broke with the big refactor that just merged.

Myoldmopar · 2024-09-10T13:01:00Z

(
And if you ever see a failure like that

You should be able to just click the Details tab and choose to re run the Jobs on the right side:

)

Anyway, pulled and it's all happy, merging this in. Thanks for your patience @Nigusse

Nigusse added 2 commits July 22, 2024 10:13

Modified HSPF2 calculation

f593a1e

Modified a fail unit test

1a05580

Nigusse added the Defect Includes code to repair a defect in EnergyPlus label Jul 22, 2024

Nigusse commented Jul 22, 2024

View reviewed changes

Nigusse added 5 commits July 22, 2024 15:06

Added new unit tests

534aa06

Set the full capacity and energy at the next higher speed level

0e01ee0

Modified the equations for capacity and energy at low speed

32ea83a

Modified the equations for capacity and energy at high speed

6b77db2

allow HSPF2 calculation with flat curve

dc265ec

Nigusse commented Jul 24, 2024

View reviewed changes

Nigusse added this to the EnergyPlus 24.2 milestone Jul 25, 2024

Nigusse added 2 commits July 25, 2024 09:39

Corrected equation re-write mistake

a0ac2ca

Merge branch 'develop' into Issue9909_multispeed_DX_Htg_Coil_HSPF2

aae4cf0

Nigusse changed the title ~~Address HSPF2 calculation problem when building load is greater than the full speed capacity for multispeed system~~ Address HSPF2 calculation problems for multispeed DX system Jul 25, 2024

Myoldmopar approved these changes Jul 25, 2024

View reviewed changes

mjwitte self-assigned this Jul 31, 2024

Nigusse added 2 commits August 2, 2024 07:44

Merge branch 'develop' into Issue9909_multispeed_DX_Htg_Coil_HSPF2

205c9f4

Updated the new unit tests due to change in develop

0008719

Nigusse mentioned this pull request Aug 12, 2024

StandardsRatings: calculation of metrics can crash if certain preconditions aren't met #10651

Closed

3 tasks

Myoldmopar added 2 commits August 16, 2024 08:29

Merge remote-tracking branch 'origin/develop' into Issue9909_multispe…

2c5368c

…ed_DX_Htg_Coil_HSPF2

Merge develop, cleanup unused vars

684d997

rraustad and others added 21 commits August 29, 2024 17:53

scope

8704f04

scope

81c66a7

scope

59f7c9d

scope

c37a243

scope

dd8d453

scope

3e19dc9

scope

4436bf4

scope

7833ec8

clang fornat

812ac1a

redundant logic

31353eb

const and reduce

f3abcc6

const and unused

9e81d19

const, scope and shadow outer function

6b3c9cc

scope

8f2a750

scope

ae2ee8f

scope and const

ffaa60e

const

52b2ae2

Move to match previous code

4648781

Remove third party nested gtest folder

eb003d1

Code changed missed during resolving merge conflict

2ae2538

Merge branch 'develop' into Issue9909_multispeed_DX_Htg_Coil_HSPF2

a0a1be5

Merge remote-tracking branch 'remotes/origin/develop' into Issue9909_…

7755930

…multispeed_DX_Htg_Coil_HSPF2

mjwitte reviewed Sep 7, 2024

View reviewed changes

mjwitte removed their assignment Sep 9, 2024

Myoldmopar merged commit 7e6ca23 into develop Sep 10, 2024
9 of 10 checks passed

Myoldmopar deleted the Issue9909_multispeed_DX_Htg_Coil_HSPF2 branch September 10, 2024 13:01

Address HSPF2 calculation problems for multispeed DX system #10618

Address HSPF2 calculation problems for multispeed DX system #10618

Conversation

Nigusse commented Jul 22, 2024 • edited by mjwitte Loading

Pull request overview

Pull Request Author

Reviewer

Nigusse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nigusse Jul 22, 2024 • edited Loading

Choose a reason for hiding this comment

Nigusse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nigusse Aug 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nigusse Jul 24, 2024 • edited Loading

Choose a reason for hiding this comment

Nigusse commented Jul 25, 2024

Nigusse commented Jul 25, 2024

Nigusse commented Jul 25, 2024

Nigusse commented Jul 25, 2024 • edited Loading

Nigusse commented Jul 25, 2024

Myoldmopar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Myoldmopar commented Aug 16, 2024

Nigusse commented Aug 16, 2024

Nigusse commented Aug 16, 2024 • edited Loading

Nigusse commented Aug 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 7, 2024

⚠️ Regressions detected on macos-14 for commit f5b88e4

Myoldmopar commented Sep 9, 2024

Myoldmopar commented Sep 10, 2024

Nigusse commented Jul 22, 2024 •

edited by mjwitte

Loading

Nigusse Jul 22, 2024 •

edited

Loading

Nigusse Aug 20, 2024 •

edited

Loading

Nigusse Jul 24, 2024 •

edited

Loading

Nigusse commented Jul 25, 2024 •

edited

Loading

Nigusse commented Aug 16, 2024 •

edited

Loading

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