Open jmarrec opened 3 years ago
The problem is that SetupStratifiedNodes
is called before Sizing has occurred (eg: SizeTankForSupplySide / SizeTankForDemandSide)
so since Tank.Height is autosized, you end up with a -9999 lurking in all calcs, which gives bogus results through, and it never initializes the Heater1Node :
* thread #1, name = 'energyplus_test', stop reason = breakpoint 4.1
frame #0: 0x00007ffff4ce9296 libenergyplusapi.so.9.4.0`EnergyPlus::WaterThermalTanks::WaterThermalTankData::SetupStratifiedNodes(this=0x0000555559462e80, state=0x0000555559ee1e60) at WaterThermalTanks.cc:5333
5330 }
5331
5332 // Assign heater elements to the nodes at the specified heights
-> 5333 if ((this->HeaterHeight1 <= H0) && (this->HeaterHeight1 > H)) {
5334 // sensor node will not get set if user enters 0 for this heater capacity
5335 // (Tank%MaxCapacity > 0.0d0)) THEN
5336 this->HeaterNode1 = NodeNum;
(lldb) p this->HeaterHeight1
(Real64) $8 = 1
(lldb) p H0
(Real64) $9 = -99999
(lldb) p H
(Real64) $10 = -83332.5
(lldb)
@nmerket FYI. Any insight to offer @jmarrec ?
Good sleuthing @jmarrec. Sorry this took so long for me to see. The "obvious" solution is to run SetupStratifiedNodes
after sizing. As long as you don't need to call CalcWaterThermalTankStratified
during sizing, that should be fine. Otherwise we'll have to come up with some sort of iteration.
Thanks @nmerket. I looked into where I could insert that call, but I'm having a hard time finding a right slot.
The Height param is set in SizeTankForSupplySide
. That's called in onInitLoopEquip
The thing is, the this->initialize
tries to call CalcStandardRatings
, which needs the nodes. Even if try to protect against it by adding a this->Height > 0
on https://github.com/NREL/EnergyPlus/blob/ad0249bc3ec63e73513039d0ac0e18e22d31649d/src/EnergyPlus/WaterThermalTanks.cc#L11397,
I still get a crash because it tries to call WaterThermalTankData::CalcWaterThermalTankStratified
while Nodes aren't initialized.
I pushed a branch where I added a unit test that exhibits the crash, and a broken WIP of one of my attempt at delaying the call. If you could nudge me in the right direction I'd highly appreciate it. If you think this would take too long, then perhaps I could transfer the pivotal ticket over to you.
https://github.com/NREL/EnergyPlus/compare/8412_Crash_AirDensity_StratifiedTank?expand=1
@nmerket Any advice for @jmarrec
I pushed a branch where I added a unit test that exhibits the crash, and a broken WIP of one of my attempt at delaying the call. If you could nudge me in the right direction I'd highly appreciate it. If you think this would take too long, then perhaps I could transfer the pivotal ticket over to you.
@jmarrec @mjwitte So sorry I dropped this. I'll pull your branch and take a crack at fixing it. Thanks for making the unit test!
@nmerket I'm going to assign this to you in Pivotal as well so it shows up properly assigned on the iteration calls.
Issue overview
user reported on unmet hours that he got the following error:
user supplied the file, which was at E+ 8.7. I updated the file to v9.4 and instead of getting this severe I am getting an actual crash, which isn't something we want to happen, even if it is later found the defect file had problems.
Details
Some additional details for this issue (if relevant):
Backtrace
Checklist
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.