Skip to content

Fix Unitary System Multispeed Heat Pump Not Propagating Error Flag #11433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mitchute wants to merge 1 commit into
base: develop
Choose a base branch
from
+14 −2
Open

Fix Unitary System Multispeed Heat Pump Not Propagating Error Flag #11433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/EnergyPlus/UnitarySystem.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ namespace UnitarySystems {
}
}

void DesignSpecMSHP::getDesignSpecMSHPdata(EnergyPlusData &state, bool errorsFound)
void DesignSpecMSHP::getDesignSpecMSHPdata(EnergyPlusData &state, bool &errorsFound)
Copy link
Collaborator

@rraustad rraustad Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed!

    DesignSpecMSHP::getDesignSpecMSHPdata(state, errorsFound);

    if (errorsFound) {
        ShowFatalError(state, "Design Specification MultiSpeed Heat Pump: Previous errors cause termination.");
    }

Copy link
Collaborator

@rraustad rraustad Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So with this fix how could the original unit test have completed without fatal'ing out?

Copy link
Collaborator Author

@mitchute mitchute Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See this comment

{
std::string const cCurrentModuleObject = "UnitarySystemPerformance:Multispeed";

Expand Down
2 changes: 1 addition & 1 deletion src/EnergyPlus/UnitarySystem.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ namespace UnitarySystems {
bool m_SingleModeFlag = false;

static void getDesignSpecMSHP(EnergyPlusData &state);
static void getDesignSpecMSHPdata([[maybe_unused]] EnergyPlusData &state, bool errorsFound);
static void getDesignSpecMSHPdata([[maybe_unused]] EnergyPlusData &state, bool &errorsFound);

DesignSpecMSHP() = default;
~DesignSpecMSHP() = default;
Expand Down
12 changes: 12 additions & 0 deletions tst/EnergyPlus/unit/UnitarySystem.unit.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26817,6 +26817,18 @@ TEST_F(ZoneUnitarySysTest, ZeroCoolingSpeedTest)
, !- Heat Recovery Water Outlet Node Name
UnitarySystemPerformance:Multispeed, !- Design Specification Multispeed Object Type
Sys 1 Furnace DX Cool Unitary System MultiSpeed Performance; !- Design Specification Multispeed Object Name

UnitarySystemPerformance:Multispeed,
MultiSpeed Performance, !- Name
1, !- Number of Speeds for Heating
2, !- Number of Speeds for Cooling
Copy link
Collaborator Author

@mitchute mitchute Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't match the name of the test ("ZeroCoolingSpeedTest"). @rraustad thoughts on whether this or the test name needs to change?

Copy link
Collaborator

@rraustad rraustad Feb 25, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well first off, the name of this new object, MultiSpeed Performance doesn't match what is used in the UnitarySystem object as Sys 1 Furnace DX Cool Unitary System MultiSpeed Performance at line 26819. So I would guess this new object isn't even used and the rest of the unit test (e.g., EXPECT_EQ) wasn't affected. I'm not exactly sure what this "ZeroCoolingSpeedTest" is testing for but I suspect if the name of the UnitarySystemPerformance:Multispeed is corrected it will quickly become apparent.

Copy link
Collaborator Author

@mitchute mitchute Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I thought I corrected that. Just checked locally, and either way, it doesn't appear to matter what the name is. Fun.

Copy link
Collaborator

@rraustad rraustad Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may not matter in this unit test, but I hope it matters in a simulation. I do see that the EXPECT_EQ's are looking for the cooling coil to be off. Whether that's a result of the multispeed performance object (obviously not) or something else is the question. I think the unit test is testing this line so adding a multispeed performance object will likely not change the result.

thisSys->m_CoolingSpeedNum = 0;

Copy link
Collaborator Author

@mitchute mitchute Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like getDesignSpecMSHPIndex raises a severe error message and returns a -1 when a matching object isn't found. However, errorsFound never gets raised. There also are a handful of places where we are doing something if the index is >-1 (i.e. we have a valid, named performance object), but then don't catch the other side of it.

Are there any legitimate cases where we don't find the object, and then just continue on? It seems to me we should either find a valid object, or fatal out. Thoughts?

Copy link
Collaborator

@rraustad rraustad Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be fair to say that if the inputs are blank then -1 is valid, otherwise a valid object should be found.

No, !- Single Mode Operation
, !- No Load Supply Air Flow Rate Ratio
AutoSize, !- Heating Speed 1 Supply Air Flow Ratio
AutoSize, !- Cooling Speed 1 Supply Air Flow Ratio
AutoSize, !- Heating Speed 2 Supply Air Flow Ratio
AutoSize; !- Cooling Speed 2 Supply Air Flow Ratio

Coil:Cooling:DX,
Sys 1 Furnace DX Cool Cooling Coil, !- Name
Sys 1 Furnace DX Cool Supply Fan Outlet, !- Evaporator Inlet Node Name
Expand Down
Loading