Followup on the assertion failure solving Te at low load condition in VRFFluidTCtrl model

[yujiex]

Pull request overview

• Fixes Followup on the assertion failure solving Te at low load condition in VRFFluidTCtrl model #10993

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 any diffs are expected, author must demonstrate they are justified using plots and descriptions
• If changes fix a defect, the fix should be demonstrated in plots and descriptions
• If any defect files are updated to a more recent version, upload new versions here or on DevSupport
• If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label
• If structural output changes, add to output rules file and add OutputChange label
• If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependencies

Reviewer

• Perform a Code Review on GitHub
• If branch is behind develop, merge develop and build locally to check for side effects of the merge
• If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
• If feature, test running new feature, try creative ways to break it
• CI status: all green or justified
• Check that performance is not impacted (CI Linux results include performance check)
• Run Unit Test(s) locally
• Check any new function arguments for performance impacts
• Verify IDF naming conventions and styles, memos and notes and defaults
• If new idf included, locally check the err file and other outputs

[github-actions]

⚠️ Regressions detected on macos-14 for commit 9dbbb6e

Regression Summary

• ERR: 4

• View Results
• Download Regressions

[github-actions]

⚠️ Regressions detected on macos-14 for commit 712847e

Regression Summary

• ERR: 4
• ESO Big Diffs: 1
• MTR Small Diffs: 1
• Table Big Diffs: 1

• View Results
• Download Regressions

[rraustad]

From the CI results it looks like this worked as expected. Fix up the unit test that failed and this should work. This is approved but will require a small change to the unit test before merging.

[yujiex]

From the CI results it looks like this worked as expected. Fix up the unit test that failed and this should work. This is approved but will require a small change to the unit test before merging.

Just fixed the unit test. Thanks @rraustad

[github-actions]

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

Regression Summary

• ERR: 4
• ESO Big Diffs: 1
• MTR Small Diffs: 1
• Table Big Diffs: 1

• View Results
• Download Regressions

[Myoldmopar]

OK, looks good now. I'll merge shortly unless there's any remaining issues.

[Myoldmopar]

OK, going in. Thanks for these last minute things @yujiex

[rraustad]

@yujiex this new warning, which is a little cryptic (i.e., the user won't know what to do to fix it), is saying why SolveRoot failed with -2. At Te = MinOutdoorUnitTe the capacity is greater than the condenser heat, and at Te = T_suction capacity is also greater than condenser heat.

+   ** Warning ** VRFOU_CalcCompC: no Te solution was found for VRF HEAT PUMP f(5)=-0.9983530605280415 and f(8.71525151555966)=-0.9985415240753804 are the same sign

The solution is trying to find Te that will match condenser heat with capacity at speed 1. That seems a little strange since how could that ever happen? I think the model is trying to find Te (SmallLoadTe) that will meet the load at comp speed = 1, without cycling the compressor. In this case that was not possible because the load was smaller than the model allowed at minimum speed/inputs. So if this system could not converge on the small load, and the SolveRoot result was -2, then shouldn't the model operate the system at MinOutdoorUnitTe and then cycle the compressor? Also, this will likely always happen with a VRF system so these warnings will always show up (maybe why they weren't reported in the first place) and an effort should be made to try to eliminate them (i.e., the warnings are only there to point developers to a problem and to inform users of a way to fix the problem if possible). What is also interesting is the warning message tells you this would happen (i.e., f(5) is negative and f(x) is negative). So if the model were to check that first, and only iterate if one of these were positive and the other negative, then the model would never result in SolFla = -2. There is no reason to call SolveRoot if at the bounds (MinOutdoorUnitTe and T_suction) both results are either negative or positive. The issue, if this were to be fixed, would be what Te to operate at when SolFla = -2 and both results were negative (I think MinOutdoorUnitTe) or both results were positive (I think T_suction or higher than T_suction). I can't tell if the model is trying to converge on compressor speed =1 or on the minimum compressor speed.

            auto f = [&state, T_discharge_new, CondHeat, CAPFT](Real64 const T_suc) {
                return CompResidual_FluidTCtrl(state, T_discharge_new, CondHeat, CAPFT, T_suc);
            };

            General::SolveRoot(state, 1.0e-3, MaxIter, SolFla, SmallLoadTe, f, MinOutdoorUnitTe,
                               T_suction); // SmallLoadTe is the updated Te'


Real64 CompResidual_FluidTCtrl

    CAPSpd = CurveValue(state, CAPFT, T_dis, T_suc); <-- is this at the speed 1 compressor speed or the minimum speed?
    CompResidual = (CondHeat - CAPSpd) / CAPSpd; <-- this is always negative

[yujiex]

@rraustad It does seem to attempt to adjust Te to meet the load at the minimum compressor speed level (maybe not speed = 1, but the speed corresponding to the smallest capacity). I guess the current situation seem to have very narrow range for Te (a lot of cases are like Te ranging from 6 to 6.5). So in many situation the solver can't find a solution.

The model does try to calculate cycling ratio after the Te adjustment (likely to just set to MinOutdoorUnitTe).

I will try to add a check of f(xmin)*f(xmax) < 0 before calling the solver. So the SolFla = -2 warning might not be needed.

CAPSpd = CurveValue(state, CAPFT, T_dis, T_suc); corresponds to the minimum speed
as

int CAPFT = this->OUCoolingCAPFT(CounterCompSpdTemp); // <- this is the index of the curve corresponding to the minimum compressor speed
...
auto f = [&state, T_discharge_new, CondHeat, CAPFT](Real64 const T_suc) {
    return CompResidual_FluidTCtrl(state, T_discharge_new, CondHeat, CAPFT, T_suc);
};

[rraustad]

@yujiex you probably want to split this up to capture what side of the solution your at:

Real64 xmin = f(xmin);
Real64 xmax = f(xmax);
if (xmin < 0 && xmax < 0) {
    SmallLoadTe = MinOutdoorUnitTe; <-- because load is less than minimum capacity
} else if (xmin > 0 && xmax > 0) {
    SmallLoadTe = ?; <-- need to figure out what to do here but I assume the solution would be at speed > 1
} else {
    SolveRoot() <-- call SolveRoot because we know it should converge
    if (SolFla == -1) {
        ShowWarningError and RecurringError
    } else if (SolFla == -2) {
        ShowWarningError and RecurringError
    }
}

[yujiex]

I see. Make sense. For the xmin > 0 && xmax > 0 case, should we just take the x where f(x) is closer to 0?

[rraustad]

I see. Make sense. For the xmin > 0 && xmax > 0 case, should we just take the x where f(x) is closer to 0?

I've thought about doing that in the past. And in that case it would be T_suction, I think. My question is, could T_suction be higher than what is used for this iteration (i.e., all that is known are the bounds MinOutdoorUnitTe and T_suction, but could the answer be higher than T_suction) ? So I really don't know the answer to your question.

[yujiex]

I feel in the original code, it was assumed that when it's ended up in the low load calculation branch, this xmin > 0 && xmax > 0 won't happen. As the code is like this, where the condition to enter this branch is Q_evap_req * C_cap_operation <= CompEvaporatingCAPSpd(1)

if (Q_evap_req * C_cap_operation <= CompEvaporatingCAPSpd(CounterCompSpdTemp)) {
    if (CounterCompSpdTemp > 1) {
        // regular load calculation
    } else { // CounterCompSpdTemp = 1, low load calculation branch <----
        Label13:;
        // C_cap_operation, T_discharge, Pipe_Q etc. changed
        if (stuff) {
            goto Label13;
        }
    }
}

But later on, this condition might have changed during the label13 part of the code

The following are debug prints: At the beginning, low load condition of Q_evap_req * C_cap_operation <= CompEvaporatingCAPSpd(1) is true, but after one round of evaluation with C_cap_operation, T_discharge, et.c updated, the condition doesn't hold any more.

07/21 08-3,beginning: Q_evap_req=11414.60122,C_cap_op=1.03810,min_spd_cap=14096.96319,lowLoad=true
07/21 08-3, f(6)=-0.023393801027263954, f(5.96607243050645)=-0.02229,Q_evap_req=11414.60122,C_cap_op=1.03810,min_spd_cap=12119.69813,lowLoad=true
go to label 13 -----------------------------
07/21 08-3, f(6)=0.07376439897663753, f(6.5)=0.05610,Q_evap_req=11413.16140,C_cap_op=1.14153,min_spd_cap=12336.37817,lowLoad=false

For the direction, sometimes it needs larger, sometimes needs smaller to have a solution

07/21 08-3, f(6)=-0.02339, f(5.96607243050645)=-0.02229, f(1)=0.15985, f(11)=-0.16870 <- need T_suction to be smaller here
07/21 08-3, f(6)=0.07376, f(6.5)=0.05610, f(1)=0.27524, f(11)=-0.08600 <- need T_suction to be larger here

[rraustad]

OK, you can give that a try. The warnings in SolFla = -1 and SolFla = -2 will show what the model is doing (make sure they each have descriptive messages). No warnings means the model is converging. Warnings in either SolFla block will show where further action is required.

[yujiex]

I added some more error message to separate the three cases
The following are the results running each of the files

--------------------------------------------------------------------------------
VariableRefrigerantFlow_FluidTCtrl_wSuppHeater_5Zone.idf
   *************  ** Warning ** Low load calculation Te solution not found as load is smaller than min-speed capacity for the whole range
   *************  **   ~~~   **   This error occurred 134858 total times;
   *************  **   ~~~   **   during Warmup 88869 times;
   *************  **   ~~~   **   during Sizing 0 times.
   *************  **   ~~~   **   Max=-2.000000  Min=-2.000000
   *************
   *************  ** Warning ** Low load calculation Te solution not found as load is larger than min-speed capacity for the whole range
Take T_suction as the updated Te
   *************  **   ~~~   **   This error occurred 377 total times;
   *************  **   ~~~   **   during Warmup 319 times;
   *************  **   ~~~   **   during Sizing 0 times.
   *************  **   ~~~   **   Max=-2.000000  Min=-2.000000

--------------------------------------------------------------------------------
VariableRefrigerantFlow_FluidTCtrl_HR_5Zone.idf
   *************  ** Warning ** Low load calculation Te solution not found as load is smaller than min-speed capacity for the whole range
   *************  **   ~~~   **   This error occurred 285052 total times;
   *************  **   ~~~   **   during Warmup 178980 times;
   *************  **   ~~~   **   during Sizing 0 times.
   *************  **   ~~~   **   Max=-2.000000  Min=-2.000000

--------------------------------------------------------------------------------
VariableRefrigerantFlow_FluidTCtrl_5Zone.idf
   *************  ** Warning ** Low load calculation Te solution not found as load is smaller than min-speed capacity for the whole range
   *************  **   ~~~   **   This error occurred 177205 total times;
   *************  **   ~~~   **   during Warmup 91884 times;
   *************  **   ~~~   **   during Sizing 0 times.
   *************  **   ~~~   **   Max=-2.000000  Min=-2.000000
   *************
   *************  ** Warning ** Low load calculation Te solution not found as load is larger than min-speed capacity for the whole range
Take T_suction as the updated Te
   *************  **   ~~~   **   This error occurred 580 total times;
   *************  **   ~~~   **   during Warmup 0 times;
   *************  **   ~~~   **   during Sizing 0 times.
   *************  **   ~~~   **   Max=-2.000000  Min=-2.000000
   *************

--------------------------------------------------------------------------------
US+SF+CZ4A+hp+crawlspace+IECC_2006_VRF.idf
   *************  ** Warning ** Low load calculation Te solution not found as load is smaller than min-speed capacity for the whole range
   *************  **   ~~~   **   This error occurred 1076321 total times;
   *************  **   ~~~   **   during Warmup 120 times;
   *************  **   ~~~   **   during Sizing 0 times.
   *************  **   ~~~   **   Max=-2.000000  Min=-2.000000
   *************
   *************  ** Warning ** Low load calculation Te solution not found as load is larger than min-speed capacity for the whole range
Take T_suction as the updated Te
   *************  **   ~~~   **   This error occurred 219531 total times;
   *************  **   ~~~   **   during Warmup 0 times;
   *************  **   ~~~   **   during Sizing 0 times.
   *************  **   ~~~   **   Max=-2.000000  Min=-2.000000
   *************

I tried taking the x such that f(x) is closer to 0 when xmin and xmax is both positive. It seems T_suction is always taken in that case.

[rraustad]

Why is the min and max always -2 in all these example files? and why doesn't T_suction float around so that -2 is not always the result?

Ahh, this should not be reporting SolFla, it should report SmallLoadTe:

               ShowRecurringWarningErrorAtEnd(state,
                                               "Low load calculation Te solution not found as end points have the same sign",
                                               this->LowLoadTeErrorIndex,
                                               SolFla,
                                               SolFla);

If T_suction is always used it makes me think that the bounds are wrong here (that's just a guess, I have no idea what this model is trying to accomplish here looking for SmallLoadTe). Maybe use MinOutdoorUnitTe as the lower bound and something like T_suction + 6 as the upper bound, so that the SolveRoot function can find a solution near "T_suction". Maybe you'll get more information once the warning min/max data are corrected.

[yujiex]

Let me change it to report SmallLoadTe in the error message. Also will try to widen the range in SolveRoot by adding some constant to the upper end point T_suction

[yujiex]

@rraustad the following are the error ouputs after changing the SolveRoot x range to MinOutdoorUnitTe to T_suction + 6. This eliminated or reduced the error counts with xmin > 0 && xmax > 0.

--------------------------------------------------------------------------------
VariableRefrigerantFlow_FluidTCtrl_wSuppHeater_5Zone.idf
   *************  ** Warning ** Low load calculation Te solution not found as load is smaller than min-speed capacity for the whole range
   *************  **   ~~~   **   This error occurred 134858 total times;
   *************  **   ~~~   **   during Warmup 88869 times;
   *************  **   ~~~   **   during Sizing 0 times.
   *************  **   ~~~   **   Max=6.500000  Min=6.500000
   *************
--------------------------------------------------------------------------------
VariableRefrigerantFlow_FluidTCtrl_HR_5Zone.idf
   *************  ** Warning ** Low load calculation Te solution not found as load is smaller than min-speed capacity for the whole range
   *************  **   ~~~   **   This error occurred 285052 total times;
   *************  **   ~~~   **   during Warmup 178980 times;
   *************  **   ~~~   **   during Sizing 0 times.
   *************  **   ~~~   **   Max=5.500000  Min=5.500000
--------------------------------------------------------------------------------
VariableRefrigerantFlow_FluidTCtrl_5Zone.idf
   *************  ** Warning ** Low load calculation Te solution not found as load is smaller than min-speed capacity for the whole range
   *************  **   ~~~   **   This error occurred 177205 total times;
   *************  **   ~~~   **   during Warmup 91884 times;
   *************  **   ~~~   **   during Sizing 0 times.
   *************  **   ~~~   **   Max=6.500000  Min=6.500000
   *************
--------------------------------------------------------------------------------
US+SF+CZ4A+hp+crawlspace+IECC_2006_VRF.idf
   *************  ** Warning ** Low load calculation Te solution not found as load is smaller than min-speed capacity for the whole range
   *************  **   ~~~   **   This error occurred 1076136 total times;
   *************  **   ~~~   **   during Warmup 120 times;
   *************  **   ~~~   **   during Sizing 0 times.
   *************  **   ~~~   **   Max=6.500000  Min=6.500000
   *************  ** Warning ** Low load calculation Te solution not found as load is larger than min-speed capacity for the whole range
Take T_suction as the updated Te
   *************  **   ~~~   **   This error occurred 29725 total times;
   *************  **   ~~~   **   during Warmup 0 times;
   *************  **   ~~~   **   during Sizing 0 times.
   *************  **   ~~~   **   Max=14.207176  Min=12.288678
   *************

[rraustad]

@yujiex, now you can see what is happening. These warnings will always occur during normal operation so you don't really need the warnings if you can figure out what to do when you know SolveRoot will fail. You could eliminate most of these warnings by using this type of logic. Then that would leave just figuring out what to do when (xmin > 0 && xmax > 0). I'm not exactly sure but using T_suction + 6 is not the best choice for the (xmin > 0 && xmax > 0) case. You do still want warnings after you call SolveRoot since you want to know if that call succeeded or not.

For these warnings, is T_suction being reported, or T_suction + 6? And for the warnings that were eliminated when xmin > 0 && xmax > 0, was SmallLoadTe higher than T_suction? I suspect that is true (which means I'm not sure how to solve this).

*************  ** Warning ** Low load calculation Te solution not found as load is larger than min-speed capacity for the whole range Take T_suction as the updated Te
*************  **   ~~~   **   This error occurred 29725 total times;
*************  **   ~~~   **   during Warmup 0 times;
*************  **   ~~~   **   during Sizing 0 times.
*************  **   ~~~   **   Max=14.207176  Min=12.288678

The other thing that I was wondering about was whether these warnings are reported once per time step, or if there were multiple warnings reported during each time step. It's misleading to report multiple warnings during each time step while the solution is trying to converge. And if the solution did converge on a given time step then you wouldn't want any warnings for that time step.

[yujiex]

@rraustad I changed to use this logic. Please see this PR: #11000

For the warnings produced in the previous round, the reported value in the warning is SmallLoadTe which ranges from MinOutdoorUnitTe to T_suction + 6. After the SolveRoot domain is enlarged to up to T_suction + 6, SmallLoadTe is indeed higher than T_suction in some cases, as the number of warnings are reduced for the case where xmin > 0 && xmax > 0. For the case where this warning is produced "Low load calculation Te solution not found as load is larger than min-speed capacity for the whole range Take T_suction as the updated Te", SmallLoadTe equals T_suction + 6