Merge pull request #10578 from NREL/enable_fe_default_debug

Enable Floating point exceptions by default in Debug mode (GCC/Clang)
NREL · Aug 16, 2024 · 2b5104f · 2b5104f · nrel-bot-2 · Aug 16, 2024
2 parents ff364d3 + 5c09ebf
commit 2b5104f
Show file tree

Hide file tree

Showing 21 changed files with 359 additions and 225 deletions.
diff --git a/cmake/CompilerFlags.cmake b/cmake/CompilerFlags.cmake
@@ -76,15 +76,19 @@ if(MSVC AND NOT ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Intel")) # Visual C++ (VS
   target_compile_options(project_options INTERFACE $<$<CONFIG:Debug>:/Ob0>) # Disable inlining
   target_compile_options(project_options INTERFACE $<$<CONFIG:Debug>:/RTCsu>) # Runtime checks
   target_compile_options(project_fp_options INTERFACE $<$<CONFIG:Debug>:/fp:strict>) # Floating point model
-  target_compile_options(project_options INTERFACE $<$<CONFIG:Debug>:/DMSVC_DEBUG>) # Triggers code in main.cc to catch floating point NaNs
-
   target_compile_options(turn_off_warnings INTERFACE /W0)
 
+  option(FORCE_DEBUG_ARITHM_MSVC "Enable trapping floating point exceptions in non Debug mode" OFF)
+  mark_as_advanced(FORCE_DEBUG_ARITHM_MSVC)
+
+  set(need_arithm_debug_genex "$<OR:$<BOOL:${FORCE_DEBUG_ARITHM_MSVC}>,$<CONFIG:Debug>>")
+
+  # in main.cc for E+ (actual: api/EnergyPlusPgm.cc) and gtest: will catch _EM_ZERODIVIDE | _EM_INVALID | _EM_OVERFLOW
+  target_compile_definitions(project_fp_options INTERFACE $<${need_arithm_debug_genex}:DEBUG_ARITHM_MSVC>)
+
 elseif(CMAKE_COMPILER_IS_GNUCXX OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "AppleClang") # g++/Clang
 
-  # TODO: after we fix all test, enable this by default on Debug builds
-  # option(FORCE_DEBUG_ARITHM_GCC_OR_CLANG "Enable trapping floating point exceptions in non Debug mode" OFF)
-  option(FORCE_DEBUG_ARITHM_GCC_OR_CLANG "Enable trapping floating point exceptions" OFF)
+  option(FORCE_DEBUG_ARITHM_GCC_OR_CLANG "Enable trapping floating point exceptions in non Debug mode" OFF)
   mark_as_advanced(FORCE_DEBUG_ARITHM_GCC_OR_CLANG)
 
   # COMPILER FLAGS
@@ -126,16 +130,13 @@ elseif(CMAKE_COMPILER_IS_GNUCXX OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" O
 
   set(need_arithm_debug_genex "$<OR:$<BOOL:${FORCE_DEBUG_ARITHM_GCC_OR_CLANG}>,$<CONFIG:Debug>>")
 
-  # TODO: after we fix all tests, remove this if statement (keeping the block to always execute) to enable this by default on Debug builds
-  if (FORCE_DEBUG_ARITHM_GCC_OR_CLANG)
-    # in main.cc for E+ and gtest: feenableexcept(FE_DIVBYZERO | FE_INVALID | FE_OVERFLOW)
-    target_compile_definitions(project_fp_options INTERFACE $<${need_arithm_debug_genex}:DEBUG_ARITHM_GCC_OR_CLANG>)
-    include(CheckCXXSymbolExists)
-    check_cxx_symbol_exists(feenableexcept "fenv.h" HAVE_FEENABLEEXCEPT)
-    message(VERBOSE "HAVE_FEENABLEEXCEPT=${HAVE_FEENABLEEXCEPT}")
-    if(HAVE_FEENABLEEXCEPT)
-      target_compile_definitions(project_fp_options INTERFACE HAVE_FEENABLEEXCEPT)
-    endif()
+  # in main.cc for E+ (actual: api/EnergyPlusPgm.cc) and gtest: feenableexcept(FE_DIVBYZERO | FE_INVALID | FE_OVERFLOW)
+  target_compile_definitions(project_fp_options INTERFACE $<${need_arithm_debug_genex}:DEBUG_ARITHM_GCC_OR_CLANG>)
+  include(CheckCXXSymbolExists)
+  check_cxx_symbol_exists(feenableexcept "fenv.h" HAVE_FEENABLEEXCEPT)
+  message(VERBOSE "HAVE_FEENABLEEXCEPT=${HAVE_FEENABLEEXCEPT}")
+  if(HAVE_FEENABLEEXCEPT)
+    target_compile_definitions(project_fp_options INTERFACE HAVE_FEENABLEEXCEPT)
   endif()
 
   # ADDITIONAL GCC-SPECIFIC FLAGS

diff --git a/src/EnergyPlus/Autosizing/HeatingCapacitySizing.cc b/src/EnergyPlus/Autosizing/HeatingCapacitySizing.cc
@@ -378,11 +378,8 @@ Real64 HeatingCapacitySizer::size(EnergyPlusData &state, Real64 _originalValue,
                 FlagCheckVolFlowPerRatedTotCap = false;
             }
 
-            if (this->dataIsDXCoil && FlagCheckVolFlowPerRatedTotCap) {
-                Real64 RatedVolFlowPerRatedTotCap = 0.0;
-                if (this->autoSizedValue > 0.0) {
-                    RatedVolFlowPerRatedTotCap = DesVolFlow / this->autoSizedValue;
-                }
+            if (this->dataIsDXCoil && FlagCheckVolFlowPerRatedTotCap && this->autoSizedValue > 0.0) {
+                Real64 RatedVolFlowPerRatedTotCap = DesVolFlow / this->autoSizedValue;
                 if (RatedVolFlowPerRatedTotCap < HVAC::MinRatedVolFlowPerRatedTotCap[(int)state.dataHVACGlobal->DXCT]) {
                     if ((this->dataEMSOverride == 0.0) && state.dataGlobal->DisplayExtraWarnings && this->printWarningFlag) {
                         ShowWarningError(state, this->callingRoutine + ' ' + this->compType + ' ' + this->compName);

diff --git a/src/EnergyPlus/CurveManager.cc b/src/EnergyPlus/CurveManager.cc
@@ -2497,6 +2497,7 @@ namespace Curve {
                         ShowSevereError(
                             state, format("Table:Lookup named \"{}\": Normalization divisor entered as zero, which is invalid", thisCurve->Name));
                         ErrorsFound = true;
+                        continue;
                     }
                 }
 

diff --git a/src/EnergyPlus/DXCoils.cc b/src/EnergyPlus/DXCoils.cc
@@ -4099,6 +4099,7 @@ void GetDXCoils(EnergyPlusData &state)
         thisDXCoil.MSRatedCOP.allocate(thisDXCoil.NumOfSpeeds);
         thisDXCoil.MSRatedAirVolFlowRate.allocate(thisDXCoil.NumOfSpeeds);
         thisDXCoil.MSRatedAirMassFlowRate.allocate(thisDXCoil.NumOfSpeeds);
+        thisDXCoil.MSRatedAirMassFlowRate = 1.0; // avoid divide by 0, will get overwritten in InitDXCoil
         thisDXCoil.MSCCapFTemp.allocate(thisDXCoil.NumOfSpeeds);
         thisDXCoil.MSCCapFFlow.allocate(thisDXCoil.NumOfSpeeds);
         thisDXCoil.MSEIRFTemp.allocate(thisDXCoil.NumOfSpeeds);
@@ -4613,6 +4614,7 @@ void GetDXCoils(EnergyPlusData &state)
         thisDXCoil.MSRatedCOP.allocate(thisDXCoil.NumOfSpeeds);
         thisDXCoil.MSRatedAirVolFlowRate.allocate(thisDXCoil.NumOfSpeeds);
         thisDXCoil.MSRatedAirMassFlowRate.allocate(thisDXCoil.NumOfSpeeds);
+        thisDXCoil.MSRatedAirMassFlowRate = 1.0; // avoid divide by 0, will get overwritten in InitDXCoil
         thisDXCoil.MSCCapFTemp.allocate(thisDXCoil.NumOfSpeeds);
         thisDXCoil.MSCCapFFlow.allocate(thisDXCoil.NumOfSpeeds);
         thisDXCoil.MSEIRFTemp.allocate(thisDXCoil.NumOfSpeeds);

diff --git a/src/EnergyPlus/SolarShading.cc b/src/EnergyPlus/SolarShading.cc
@@ -415,7 +415,6 @@ void GetShadowingInput(EnergyPlusData &state)
     int NumNumbers;
     int NumAlphas;
     int IOStat;
-    int Found = 0;
     auto &cCurrentModuleObject = state.dataIPShortCut->cCurrentModuleObject;
     state.dataIPShortCut->rNumericArgs({1, 4}) = 0.0; // so if nothing gotten, defaults will be maintained.
     state.dataIPShortCut->cAlphaArgs(1) = "";
@@ -2587,7 +2586,6 @@ void AnisoSkyViewFactors(EnergyPlusData &state)
     Real64 Epsilon;                // Sky clearness parameter
     Real64 Delta;                  // Sky brightness parameter
     Real64 CosIncAngBeamOnSurface; // Cosine of incidence angle of beam solar on surface
-    Real64 IncAng;                 // Incidence angle of beam solar on surface (radians)
     int EpsilonBin;                // Sky clearness (Epsilon) bin index
     Real64 AirMass;                // Relative air mass
     Real64 AirMassH;               // Intermediate variable for relative air mass calculation
@@ -2652,8 +2650,6 @@ void AnisoSkyViewFactors(EnergyPlusData &state)
             CosIncAngBeamOnSurface = -1.0;
         }
 
-        IncAng = std::acos(CosIncAngBeamOnSurface);
-
         ViewFactorSkyGeom = state.dataSurface->Surface(SurfNum).ViewFactorSky;
         state.dataSolarShading->SurfMultIsoSky(SurfNum) = ViewFactorSkyGeom * (1.0 - F1);
         //           0.0871557 below corresponds to a zenith angle of 85 deg
@@ -3141,19 +3137,17 @@ bool polygon_contains_point(int const nsides,            // number of sides (ver
     // Using/Aliasing
     using namespace DataVectorTypes;
 
-    // Return value
-    bool inside; // return value, true=inside, false = not inside
+    // return value, true=inside, false = not inside
 
     EP_SIZE_CHECK(polygon_3d, nsides);
 
-    int i;
     int ip1;
 
     // Object Data
     Array1D<Vector_2d> polygon(nsides);
     Vector_2d point;
 
-    inside = false;
+    bool inside = false;
     if (ignorex) {
         for (int i = 1; i <= nsides; ++i) {
             polygon(i).x = polygon_3d(i).y;
@@ -3180,7 +3174,7 @@ bool polygon_contains_point(int const nsides,            // number of sides (ver
         point.x = point.y = 0.0; // Elim possibly used uninitialized warnings
     }
 
-    for (i = 1; i <= nsides; ++i) {
+    for (int i = 1; i <= nsides; ++i) {
 
         if (i < nsides) {
             ip1 = i + 1;

diff --git a/src/EnergyPlus/VariableSpeedCoils.cc b/src/EnergyPlus/VariableSpeedCoils.cc
@@ -5556,7 +5556,11 @@ namespace VariableSpeedCoils {
             RatedInletEnth = Psychrometrics::PsyHFnTdbW(RatedInletAirTemp, RatedInletAirHumRat);
             CBFRated = AdjustCBF(varSpeedCoil.MSRatedCBF(NormSpeed), varSpeedCoil.MSRatedAirMassFlowRate(NormSpeed), RatedAirMassFlowRate);
             if (CBFRated > 0.999) CBFRated = 0.999;
-            AirMassFlowRatio = RatedAirMassFlowRate / varSpeedCoil.MSRatedAirMassFlowRate(NormSpeed);
+            if (varSpeedCoil.MSRatedAirMassFlowRate(NormSpeed) > 1.0e-10) {
+                AirMassFlowRatio = RatedAirMassFlowRate / varSpeedCoil.MSRatedAirMassFlowRate(NormSpeed);
+            } else {
+                AirMassFlowRatio = 1.0;
+            }
 
             if (varSpeedCoil.MSRatedWaterVolFlowRate(NormSpeed) > 1.0e-10) {
                 WaterMassFlowRatio = varSpeedCoil.RatedWaterVolFlowRate / varSpeedCoil.MSRatedWaterVolFlowRate(NormSpeed);
@@ -8622,7 +8626,11 @@ namespace VariableSpeedCoils {
             Real64 tADP = PsyTsatFnHPb(state, hADP, Pressure, RoutineName); // Apparatus dew point temperature [C]
             Real64 wADP = PsyWFnTdbH(state, tADP, hADP, RoutineName);       // Apparatus dew point humidity ratio [kg/kg]
             Real64 hTinwADP = PsyHFnTdbW(InletDryBulb, wADP);               // Enthalpy at inlet dry-bulb and wADP [J/kg]
-            SHRCalc = min((hTinwADP - hADP) / (InletEnthalpy - hADP), 1.0); // temporary calculated value of SHR
+            if (TotCapCalc > 1.0e-10) {
+                SHRCalc = min((hTinwADP - hADP) / (InletEnthalpy - hADP), 1.0); // temporary calculated value of SHR
+            } else {
+                SHRCalc = 1.0;
+            }
 
             //   Check for dry evaporator conditions (win < wadp)
             if (wADP > InletHumRatCalc || (Counter >= 1 && Counter < MaxIter)) {

diff --git a/src/EnergyPlus/api/EnergyPlusPgm.cc b/src/EnergyPlus/api/EnergyPlusPgm.cc
@@ -182,10 +182,12 @@
 #include <string>
 #include <vector>
 
-#ifndef NDEBUG
-#ifdef __unix__
-#include <cfenv>
+#ifdef DEBUG_ARITHM_GCC_OR_CLANG
+#include <EnergyPlus/fenv_missing.h>
 #endif
+
+#ifdef DEBUG_ARITHM_MSVC
+#include <cfloat>
 #endif
 
 // ObjexxFCL Headers
@@ -248,17 +250,22 @@ void commonInitialize(EnergyPlus::EnergyPlusData &state)
     // std::cin.tie(nullptr); // Untie cin and cout: Could cause odd behavior for interactive prompts
 
 // Enable floating point exceptions
-#ifndef NDEBUG
-#ifdef __unix__
+#ifdef DEBUG_ARITHM_GCC_OR_CLANG
     feenableexcept(FE_DIVBYZERO | FE_INVALID | FE_OVERFLOW); // These exceptions are enabled (FE_INEXACT and FE_UNDERFLOW will not throw)
 #endif
-#endif
 
-#ifdef MSVC_DEBUG
-    // the following line enables NaN detection in Visual Studio debug builds. See
+#ifdef DEBUG_ARITHM_MSVC
+    // the following enables NaN detection in Visual Studio debug builds. See
     // https://github.com/NREL/EnergyPlus/wiki/Debugging-Tips
-    int fp_control_state =
-        _controlfp(_EM_INEXACT | _EM_UNDERFLOW, _MCW_EM); // These exceptions are disabled (_EM_INEXACT and _EM_UNDERFLOW will not throw)
+
+    // Note: what you need to pass to the _controlfp_s is actually the opposite
+    // By default all bits are 1, and the exceptions are turned off, so you need to turn off the bits for the exceptions you want to enable
+    // > For the _MCW_EM mask, clearing it sets the exception, which allows the hardware exception; setting it hides the exception.
+    unsigned int fpcntrl = 0;
+    _controlfp_s(&fpcntrl, 0, 0);
+    unsigned int new_exceptions = _EM_ZERODIVIDE | _EM_INVALID | _EM_OVERFLOW;
+    unsigned int new_control = fpcntrl & ~new_exceptions;
+    _controlfp_s(&fpcntrl, new_control, _MCW_EM);
 #endif
 
 #ifdef _MSC_VER

diff --git a/src/EnergyPlus/main.cc b/src/EnergyPlus/main.cc
@@ -50,16 +50,8 @@
 
 #include <CLI/CLI11.hpp>
 
-#ifdef DEBUG_ARITHM_GCC_OR_CLANG
-#include <EnergyPlus/fenv_missing.h>
-#endif
-
 int main(int argc, char **argv)
 {
-#ifdef DEBUG_ARITHM_GCC_OR_CLANG
-    feenableexcept(FE_DIVBYZERO | FE_INVALID | FE_OVERFLOW);
-#endif
-
 #ifdef _WIN32
     const std::vector<std::string> args = CLI::detail::compute_win32_argv();
 #else

diff --git a/tst/EnergyPlus/unit/AirflowNetworkHVAC.unit.cc b/tst/EnergyPlus/unit/AirflowNetworkHVAC.unit.cc
@@ -2212,7 +2212,7 @@ TEST_F(EnergyPlusFixture, AirflowNetwork_TestPressureStat)
 
     // Check indoor pressure and mass flow rate
     EXPECT_NEAR(PressureSet, state->afn->AirflowNetworkNodeSimu(3).PZ, 0.0001);
-    EXPECT_NEAR(0.00255337, state->afn->ReliefMassFlowRate, 0.0001);
+    EXPECT_NEAR(0.06551, state->afn->ReliefMassFlowRate, 0.0001);
 
     // Start a test for #5687 to report zero values of AirflowNetwork:Distribution airflow and pressure outputs when a system is off
     state->afn->AirflowNetworkFanActivated = false;