Fix 11261 floating point overflow in CalcEffectiveSHR#11288
Conversation
|
Looks reasonable. Compiler explorer: https://godbolt.org/z/shfP4M3qE |
| To2 = aa - Tcl * std::expm1(-To1 / Tcl); | ||
| // Floating overflow errors occur when -To1/Tcl is a large positive number. | ||
| // Cap upper limit at 700 to avoid the overflow errors. | ||
| To2 = aa - Tcl * std::expm1(min(700.0, -To1 / Tcl)); |
There was a problem hiding this comment.
@kevin-moos are you seeing an overflow or underflow? I have seen this limit (-700) used but for an underflow condition here and here. Tcl is the latent time constant which is a positive number. How did this calculation result in a large positive number? We do protect against large positive exponents (just search for 700) but not for this type of calculation. Please clarify.
There was a problem hiding this comment.
It was an overflow. I noted in the issue that it only seems to happen when EnteringDB and EnteringWB are both negative (someone was testing cold climate heat pumps), so the sign is getting flipped somewhere along the way. I saw the fix for the underflow and used that as the basis for my change.
There was a problem hiding this comment.
Is the fan operating as constant when this happens? The unit test shows cycling fan. The model is not supposed to be active during cycling fan.
| // Real64 constexpr QLatRated = 197242.22114414035; | ||
| // Real64 constexpr QLatActual = 149769.74983225350; | ||
| // Real64 constexpr EnteringDB = -4.5144144221489846; | ||
| // Real64 constexpr EnteringWB = -4.5144119262695312; |
There was a problem hiding this comment.
So maybe this is the clue. The coil turned on in winter? and caused a large positive exponent? I see cycling fan operating mode but the latent degradation model is only active for constant fan mode?
Pull request overview
Description of the purpose of this PR
Fix floating point overflow due to passing of large values to std::exp and std::expm1. This was originally discovered in VariableSpeedCoils, but I found and updated similar code in other coils as well. The unit test is only for VariableSpeedCoils.
Pull Request Author
Reviewer