New Formatting Fatal Error Confusion

[Myoldmopar]

Issue overview

With the new formatting library, invalid format calls are handled nicely in a try catch block, and a severe error is issued, but no fatal is thrown. We just need to issue the severe and then call a fatal, or just call the fatal directly with the error message:

   ************* Testing Individual Return Air Path Integrity
   ************* All Return Air Paths passed integrity testing
   ************* No node connection errors were found.
   ************* Beginning Simulation
   ** Severe  ** Error with format, '('! <VAV DX Cooling Coil Standard Rating Information>, DX Coil Type, DX Coil Name, Fan Type, Fan Name, ','Standard Net Cooling Capacity {W}, Standard Net Cooling Capacity {Btu/h}, IEER {Btu/W-h}, ','COP 100% Capacity {W/W}, COP 75% Capacity {W/W}, COP 50% Capacity {W/W}, COP 25% Capacity {W/W}, ','EER 100% Capacity {Btu/W-h}, EER 75% Capacity {Btu/W-h}, EER 50% Capacity {Btu/W-h}, EER 25% Capacity {Btu/W-h}, ','Supply Air Flow 100% {kg/s}, Supply Air Flow 75% {kg/s},Supply Air Flow 50% {kg/s},Supply Air Flow 25% {kg/s}')
', passed 0 args
   ************* EnergyPlus Warmup Error Summary. During Warmup: 0 Warning; 0 Severe Errors.
   ************* EnergyPlus Sizing Error Summary. During Sizing: 0 Warning; 0 Severe Errors.
   ************* EnergyPlus Terminated--Fatal Error Detected. 0 Warning; 1 Severe Errors; Elapsed Time=00hr 00min  1.11sec

[jmarrec]

https://github.com/NREL/EnergyPlus/blob/3d2145e57854707476a667f73c514417625964aa/src/EnergyPlus/OutputFiles.cc#L428-L451

[mjwitte]

@Myoldmopar Hmmm, that seems dangerous. Users could trip error messages that aren't intended to be fatal but have format errors lurking in them. Seems this should write a "report this as an issue" message and continue.

[Myoldmopar]

I agree. We shouldn't let this cause a simulation to fatal. The fmt::format_error should be caught, as it currently is, but then instead of raising another format error, we should issue a warning as you suggested @mjwitte and continue.

@lefticus can you address this in your next IO change? Basically just change the nested throw into a ShowWarning with the same content being passed in.

[lefticus]

@Myoldmopar can do. I have a PR in the works right now I can push it to.

[mjwitte]

This was addressed in #7997 commit https://github.com/NREL/EnergyPlus/commit/8cc27ebbcaaade6c3561550461ab7bd0ebe8062c. Closing

[mjwitte]

@Myoldmopar @mitchute So, the changes we discussed this morning were done in https://github.com/NREL/EnergyPlus/commit/8cc27ebbcaaade6c3561550461ab7bd0ebe8062c but somewhere along the way they got stomped on?

[mitchute]

Oh fun...

https://github.com/NREL/EnergyPlus/blame/f3c53608b34bb46659b7099ce3d6373db597dcbb/src/EnergyPlus/IOFiles.cc

Copy-paste mistake probably...

[mitchute]

Actually... IDK what happened. It's my fault either way.

[jmarrec]

state isn't passed in vprint, yet it's needed for ShowWarningError

https://github.com/NREL/EnergyPlus/blob/0c9b8c4944b5f06edb000bf0d766f067830af0ae/src/EnergyPlus/IOFiles.hh#L801-L822

[jmarrec]

If we don't want to throw EnergyPlus::FataError, I suppose we can just write to the ostream (/ std::string return) that message fmt::format("Error with format, '{}', passed {} args", format_str, sizeof...(Args)). Typically that'll occurr inside a call to ShowWarningError or else error so it should end up in the right place.