Skip to content
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.

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

Enable Floating point exceptions by default in Debug mode (GCC/Clang) #10578

Merged
merged 25 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
dcd3358
Enable Floating point exceptions by default in Debug mode (?)
jmarrec Jun 19, 2024
0146701
Fix RefrigeratedRackWithCaseInZone by initializing OutBaroPress
jmarrec Aug 9, 2024
b43c6d5
CurveManager: when divisor is zero, don't try to exercise it
jmarrec Aug 9, 2024
268de04
Fix CO2ControlDesignOccupancyTest
jmarrec Aug 9, 2024
40cb547
Fix WindowManager_RefAirTempTest by properly allocating arrays
jmarrec Aug 9, 2024
6686e0c
Work around for DXCoils_Test1
jmarrec Aug 9, 2024
409f804
Fix HPWHSizing
jmarrec Aug 9, 2024
7d4e4fa
Fix VariableSpeedCoils_ZeroRatedCoolingCapacity_Test
jmarrec Aug 9, 2024
d54533c
Fix SolarShadingTest_CalcBeamSolarOnWinRevealSurface
jmarrec Aug 9, 2024
8945549
Fix ManageElectricPowerTest_CheckOutputReporting
jmarrec Aug 9, 2024
0834394
Fix UnitaryBypassVAV_ParentElectricityRateTest
jmarrec Aug 9, 2024
9cdb3e4
Fix HVACMSHP_UnitarySystemElectricityRateTest
jmarrec Aug 9, 2024
a0b20cc
Similar fix for PTHP test: init the RatedAirMassFlowRate due to sizin…
jmarrec Aug 12, 2024
e92b7e8
Fix TestReadingCoilCoolingHeatingDX
jmarrec Aug 12, 2024
30fdefb
Fix SingleSpeedDXCoolingCoilOutputTest
jmarrec Aug 12, 2024
9fa8ee0
Fix MultiSpeedDXCoolingCoilOutputTest
jmarrec Aug 12, 2024
2dee72c
Fix SQLiteFixture.DXCoils_TestComponentSizingOutput_SingleSpeed / DXC…
jmarrec Aug 12, 2024
023b7f8
Fix EnergyPlusFixture.TwoSpeedDXCoilStandardRatings_Curve_Fix_Test E…
jmarrec Aug 12, 2024
64018cf
Catch floating point exceptions on MSVC for gtest + add cmake `FORCE_…
jmarrec Aug 13, 2024
6cf3c3e
Merge remote-tracking branch 'origin/develop' into enable_fe_default_…
jmarrec Aug 13, 2024
b6be21a
Fix 5 DX coil unit tests
rraustad Aug 13, 2024
7220b7b
Fixed crash, now get diffs in solar shading
rraustad Aug 14, 2024
aa433a7
clang format
jmarrec Aug 14, 2024
bbfe61b
Use relatistic SUNCOSTS
jmarrec Aug 14, 2024
5c09ebf
Merged develop; resolved conflicts
Myoldmopar Aug 14, 2024
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
31 changes: 16 additions & 15 deletions cmake/CompilerFlags.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
7 changes: 2 additions & 5 deletions src/EnergyPlus/Autosizing/HeatingCapacitySizing.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/EnergyPlus/CurveManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/EnergyPlus/DXCoils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
16 changes: 8 additions & 8 deletions src/EnergyPlus/SolarShading.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3947,11 +3947,11 @@ void CLIPLINE(Real64 &x1, Real64 &x2, Real64 &y1, Real64 &y2, Real64 maxX, Real6
else if (c2 == 3) {
visible = true;
x1 = minX;
y1 = maxY + e / dx;
y1 = maxY + General::SafeDivide(e, dx);
Copy link
Contributor

Choose a reason for hiding this comment

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

There may be a better way to correct this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if (x2 <= maxX && y2 <= maxY) return;
} else if (c2 == 4) {
x2 = maxX;
y2 = maxY + e / dx;
y2 = maxY + General::SafeDivide(e, dx);
return;
}
if (needX) {
Expand All @@ -3965,11 +3965,11 @@ void CLIPLINE(Real64 &x1, Real64 &x2, Real64 &y1, Real64 &y2, Real64 maxX, Real6
return;
else if (c2 == 1) {
visible = true;
x1 = maxX - e / dy;
x1 = maxX - General::SafeDivide(e, dy);
y1 = minY;
if (x2 <= maxX && y2 <= maxY) return;
} else if (c2 == 4) {
x2 = maxX - e / dy;
x2 = maxX - General::SafeDivide(e, dy);
y2 = maxY;
return;
}
Expand All @@ -3989,11 +3989,11 @@ void CLIPLINE(Real64 &x1, Real64 &x2, Real64 &y1, Real64 &y2, Real64 maxX, Real6
else if (c2 == 3) {
visible = true;
x1 = minX;
y1 = minY + e / dx;
y1 = minY + General::SafeDivide(e, dx);
if (x2 <= maxX && y2 >= minY) return;
} else if (c2 == 4) {
x2 = maxX;
y2 = minY + e / dx;
y2 = minY + General::SafeDivide(e, dx);
return;
}
if (needX) {
Expand All @@ -4007,11 +4007,11 @@ void CLIPLINE(Real64 &x1, Real64 &x2, Real64 &y1, Real64 &y2, Real64 maxX, Real6
return;
else if (c2 == 7) {
visible = true;
x1 = maxX - e / dy;
x1 = maxX - General::SafeDivide(e, dy);
y1 = maxY;
if (x2 <= maxX && y2 >= minY) return;
} else if (c2 == 4) {
x2 = maxX - e / dy;
x2 = maxX - General::SafeDivide(e, dy);
y2 = minY;
return;
}
Expand Down
12 changes: 10 additions & 2 deletions src/EnergyPlus/VariableSpeedCoils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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)) {
Expand Down
27 changes: 17 additions & 10 deletions src/EnergyPlus/api/EnergyPlusPgm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions src/EnergyPlus/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tst/EnergyPlus/unit/AirflowNetworkHVAC.unit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume since StdRhoAir is now in the fixture that something has changed BEFORE this unit test sets StdRhoAir.


// 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;
Expand Down
Loading
Loading