Repository navigation
Conversation
jmarrec
added
the
Performance
| option(ENABLE_NATIVE_OPTIMIZATION "Enable native architecture optimizations (e.g. -march=native). Produces non-portable binaries." OFF) | ||
| mark_as_advanced(ENABLE_NATIVE_OPTIMIZATION) |
| if(ENABLE_NATIVE_OPTIMIZATION) | ||
| # MSVC has no compiler-time CPU auto-detection equivalent to -march=native; the user must pick the level. | ||
| set(MSVC_NATIVE_ARCH "AVX2" CACHE STRING "MSVC /arch: level used when ENABLE_NATIVE_OPTIMIZATION=ON (SSE2, SSE4.2, AVX, AVX2, AVX512, AVX10.1, AVX10.2)") | ||
| set_property(CACHE MSVC_NATIVE_ARCH PROPERTY STRINGS "SSE2" "SSE4.2" "AVX" "AVX2" "AVX512" "AVX10.1" "AVX10.2") | ||
| mark_as_advanced(MSVC_NATIVE_ARCH) | ||
|
|
||
| message(STATUS "ENABLE_NATIVE_OPTIMIZATION: enabling /arch:${MSVC_NATIVE_ARCH}") | ||
| target_compile_options(project_options INTERFACE /arch:${MSVC_NATIVE_ARCH}) | ||
| endif() |
There was a problem hiding this comment.
On MSVC and Not Inel, if ENABLE_NATIVE_OPTIMIZATION is ON, another CMake variable is needed.
| if(ENABLE_NATIVE_OPTIMIZATION) | ||
| if(CMAKE_SYSTEM_PROCESSOR MATCHES "aarch64|arm64") | ||
| # -mcpu=native sets both arch and tune; -march=native alone doesn't tune on AArch64 in Clang | ||
| # (GCC 13+ maps -march=native to -mcpu=native automatically, but -mcpu=native is correct for all versions) | ||
| message(STATUS "ENABLE_NATIVE_OPTIMIZATION: enabling -mcpu=native (AArch64)") | ||
| target_compile_options(project_options INTERFACE -mcpu=native) | ||
| else() | ||
| message(STATUS "ENABLE_NATIVE_OPTIMIZATION: enabling -march=native") | ||
| target_compile_options(project_options INTERFACE -march=native) | ||
| endif() |
There was a problem hiding this comment.
GCC/Clang/AppleClang: enable -march=native or -mcpu=native (arm64)
| # Enable fast-math for vectorization; keep -fno-finite-math-only so NaN/Inf remain valid. | ||
| # -ffp-contract=off is intentionally skipped below when this option is ON. | ||
| message(STATUS "ENABLE_NATIVE_OPTIMIZATION: enabling -ffast-math -fno-finite-math-only") | ||
| target_compile_options(project_options INTERFACE -ffast-math -fno-finite-math-only) |
There was a problem hiding this comment.
-ffast-math is a bundle of several independent flags, and a bunch of them are actually needed for the native optimizations to be allowed by the compiler.
| Flag | What it does |
|---|---|
-fassociative-math |
allows reordering FP ops — this is what unlocks loop vectorization |
-ffp-contract=fast |
enables FMA (so it's a superset of -ffp-contract=fast) |
-ffinite-math-only |
assumes no NaN/Inf exist |
-fno-signed-zeros |
treats -0 == +0 |
-fno-trapping-math |
assumes FP exceptions never trap |
-fno-math-errno |
doesn't set errno after math calls |
-freciprocal-math |
replaces x/y with x*(1/y) |
The dangerous ones for a simulation code are -ffinite-math-only (EnergyPlus definitely produces NaNs as error signals in some paths).
| if(NOT ENABLE_NATIVE_OPTIMIZATION) | ||
| target_compile_options(project_fp_options INTERFACE -ffp-contract=off) # Disable fused-floating point operations (default is fast) | ||
| endif() |
There was a problem hiding this comment.
ffp-contract=off is what guarantees we get the same regression results, and more impotantly unit test floating point equality, on x64 and arm64 alike.
We do that only if ENABLE_NATIVE_OPTIMIZATION is NOT ON.
| if(ENABLE_NATIVE_OPTIMIZATION) | ||
| string(APPEND BUILD_PLATFORM_STRING " With Native Optimizations") | ||
| endif() |
There was a problem hiding this comment.
./Products/energyplus --help
EnergyPlus, Version 26.2.0-8b943ec7c8, YMD=2026.09.25 11:01
PythonLinkage: Linked to Python Version: "3.12.2 (main, Sep 9 2024, 14:39:25) [GCC 13.2.0]"
-Built on Platform: Ubuntu24.04_x86_64
+Built on Platform: Ubuntu24.04_x86_64 With Native Optimizations
Usage: ./Products/energyplus [OPTIONS] [input_file] [SUBCOMMAND]| add_executable(energyplus main.cc "${CMAKE_CURRENT_BINARY_DIR}/energyplus.rc") | ||
| endif() | ||
| target_link_libraries(energyplus PRIVATE cli11 energyplusapi project_options project_fp_options project_warnings) | ||
| target_compile_options(energyplus PRIVATE ${ENERGYPLUS_RECORD_SWITCHES_FLAGS}) |
There was a problem hiding this comment.
$ readelf --string-dump=.GCC.command.line Products/energyplus
String dump of section '.GCC.command.line':
[ 0] GNU C++20 13.3.0 -march=znver1 -mmmx -mpopcnt -msse -msse2 -msse3 -mssse3 -msse4.1 -msse4.2 -mavx -mavx2 -msse4a -mno-fma4 -mno-xop -mfma -mno-avx512f -mbmi -mbmi2 -maes -mpclmul -mno-avx512vl -mno-avx512bw -mno-avx512dq -mno-avx512cd -mno-avx512er -mno-avx512pf -mno-avx512vbmi -mno-avx512ifma -mno-avx5124vnniw -mno-avx5124fmaps -mno-avx512vpopcntdq -mno-avx512vbmi2 -mno-gfni -mno-vpclmulqdq -mno-avx512vnni -mno-avx512bitalg -mno-avx512bf16 -mno-avx512vp2intersect -mno-3dnow -madx -mabm -mno-cldemote -mclflushopt -mno-clwb -mclzero -mcx16 -mno-enqcmd -mf16c -mfsgsbase -mfxsr -mno-hle -msahf -mno-lwp -mlzcnt -mmovbe -mno-movdir64b -mno-movdiri -mmwaitx -mno-pconfig -mno-pku -mno-prefetchwt1 -mprfchw -mno-ptwrite -mno-rdpid -mrdrnd -mrdseed -mno-rtm -mno-serialize -mno-sgx -msha -mno-shstk -mno-tbm -mno-tsxldtrk -mno-vaes -mno-waitpkg -mno-wbnoinvd -mxsave -mxsavec -mxsaveopt -mxsaves -mno-amx-tile -mno-amx-int8 -mno-amx-bf16 -mno-uintr -mno-hreset -mno-kl -mno-widekl -mno-avxvnni -mno-avx512fp16 -mno-avxifma -mno-avxvnniint8 -mno-avxneconvert -mno-cmpccxadd -mno-amx-fp16 -mno-prefetchi -mno-raoint -mno-amx-complex --param=l1-cache-size=32 --param=l1-cache-line-size=64 --param=l2-cache-size=512 -mtune=znver1 -O3 -std=c++20 -fPIE -fvisibility=hidden -ffast-math -fno-finite-math-only -fno-stack-protector -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection
| // ATMixer flow rate = 0 so oaFrac depends on ZoneEqSizing.OAVolFlow | ||
| oaFrac = sizer.setOAFracForZoneEqSizing(*state, DesMassFlow, zoneEqSizing); | ||
| EXPECT_EQ(oaFrac, oaFrac_Test); | ||
| EXPECT_DOUBLE_EQ(oaFrac, oaFrac_Test); |
There was a problem hiding this comment.
Unit test changes are mostly this.
| namespace { | ||
| // The "no formatting" JSON output path (RealToStr with formatReals=false) prints the shortest | ||
| // round-trip representation of the raw double, so it is sensitive to last-bit differences (eg. | ||
| // from -ffast-math). Compare the numeric value of each cell instead of the literal string. | ||
| void expectNumericRowsNear(const ResultsFramework::json &row, const std::vector<std::string> &expected, Real64 tol = 1e-9) | ||
| { | ||
| std::vector<std::string> rowStrs = row.get<std::vector<std::string>>(); | ||
| ASSERT_EQ(expected.size(), rowStrs.size()); | ||
| for (size_t i = 0; i < expected.size(); ++i) { | ||
| EXPECT_NEAR(std::stod(expected[i]), std::stod(rowStrs[i]), tol) << "Mismatch at column " << i; | ||
| } | ||
| } | ||
| } // namespace |
There was a problem hiding this comment.
I had some test failures here with native optimizations because it's string based. So I used a helper
| for (int i = 1; i <= nsp; ++i) { | ||
| Real64 ratioAtithSpeed = (RatedTotalCapacity(i) / _100PercentCoolCap) * 100; | ||
| ratioArray.push_back(ratioAtithSpeed); | ||
| } | ||
| for (int i = 1; i <= nsp; ++i) { | ||
| if ((int)(ratioArray(i)) == 100.0) { | ||
| if (std::lround(ratioArray(i)) == 100) { |
There was a problem hiding this comment.
Fix segfault in IEERCalculationVariableSpeed from fast-math reciprocal approximation
Speed-bucket matching truncated a ratio to int and compared against 100/75/50/25, relying on x/x == 1.0 exactly. Under -freciprocal-math that division can land at 0.999999..., truncating to 99 instead of 100, leaving speedsForA empty and segfaulting on the next indexed access. Round instead of truncate.
Reliability/Accuracy of resultsObviously, if you do use But if you check Total site energy, it matches just fine. I have only 2 cases where the diff is concerning.
It gets that number per case from whichever source is available and most precise: the raw For the two where I have a diff: I briefly looked at why, and stopped when I was satisfied instead of trying to track down the specific routine/line where it happens. Looking at each test's
Both tests are literally about control logic (component setpoint control; simultaneous heating/cooling chiller-heater staging). The diff being concentrated in one specific design-day period rather than spread evenly across the whole run is the signature of a threshold comparison flipping, not accumulated rounding noise — if it were just FP precision drift, you'd expect a similar small percentage in both design days, not near-zero in one and 7-12% in the other. Instead, something like a setpoint reset schedule, chiller/heat-recovery staging condition, or simultaneous-heating/cooling mode selector is evaluating a comparison against a boundary value that |
Runtime benefitsFirst and foremost, this is absolutely and completely machine/CPU dependent. My results on a Ryzen 1800X will heavily differ from a more recent Intel/AMD chip with wider SIMD units (AVX2/AVX-512). My CPU is Anyways, the results are a bit... unexciting on my CPU. It's possible that the gains might be bigger on a different CPU and different compiler. But I suspect part of it is also that the codebase itself doesn't have many loops shaped in a way the compiler can auto-vectorize — most of the hot paths are scalar per-timestep/per-zone calculations with branches and function calls inside, not the tight numeric loops over contiguous arrays that Methodology
I ran ALL integration tests for two builds on the same machine, same flags otherwise, same SSD
Each test is run 5 times per build and averaged (828 tests, 5 reps each, ~3.3 hours total) Results
* 41 tests excluded: tests requiring the separate Basement/Slab Fortran preprocessor ( Distribution across all 787 tests: 546 measurably faster, 74 measurably slower, 167 within noise (±0.5%). The gains concentrate in the heavier/longer-running models rather than being uniform — e.g.:
A handful of tests got slower (up to +17%). These are real and reproducible (tight stdev across reps, order-independent), but I couldn't pin down a specific cause for them the way I could for the biggest speedup outlier below — One outlier is worth calling out specifically: |
…counters, and a script to compare results Offset8/16/32 view a 128-byte aligned buffer from +8/+16/+32 bytes so the alignment does not depend on the allocator (Offset16 is what std::vector<double> gets on glibc/MSVC). Sizes are 37, 251, 1003, 30011, 4000037 doubles: powers of two hid vector-loop tails and caused cache set aliasing (the Stencil -27% at 4 Mi). compare_alignment_results.py prints Markdown tables relative to Aligned64 from one or two (default, native) result files. ``` Products/bench_alignment_vectorization --benchmark_repetitions=5 --benchmark_report_aggregates_only=true --benchmark_out=../results/alignment_default.json --benchmark_out_format=json Products/bench_alignment_vectorization_native --benchmark_repetitions=5 --benchmark_report_aggregates_only=true --benchmark_out=../results/alignment_native.json --benchmark_out_format=json python3 compare_alignment_results.py results/alignment_default.json results/alignment_native.json --offsets ``` Results on two machines: macOS M1 Max (Apple clang, NEON 16-byte vectors), and Linux Ryzen 7 1800X (Zen 1, GCC, AVX2 32-byte vectors). Median of 5, max coefficient of variation 8.7% (M1) / 5.2% (Ryzen). What the allocator actually gives std::vector<double> (Default layout, observed): glibc (Ryzen) 16-byte aligned at all sizes (offsets 0/16/32/48/80/96/112 mod 128). macOS 16-byte guaranteed for small blocks, page-aligned for large ones (>= ~32 KiB, 16 KiB pages). __STDCPP_DEFAULT_NEW_ALIGNMENT__ is 16 on Apple clang, GCC and MSVC. Alignment, % vs Aligned64 (ie OBJEXXFCL_ALIGN=64): | Layout | M1 Max | Ryzen 1800X | |----------------------------------------|-----------------------------------------|-------------------------------------------| | Default / Aligned128 | within noise | within noise | | Offset16 (glibc/MSVC std::vector) | within noise, both builds | within noise, both builds (*) | | Offset32 (AVX2-aligned, not 64) | within noise | within noise (*) | | Offset8, Axpy, default / native build | +12 to +16% / +11 to +22% (n 251-30011) | +17 to +28% / +14 to +34% (n 37-30011) | | Offset8, Cubic | up to +8% | +6 to +10% (native) | (*) Two tiny-size anomalies on the Ryzen that are not alignment driven: Cubic native n=37 is +60/+70% for Offset16 and Default but also +58% for Aligned128 and +1% for Offset8; Stencil default n=37 Offset32 +17%. Most likely relative placement of input/output buffers (store/load address aliasing), not 16 vs 64. Native (-march/-mcpu=native -ffast-math -fno-finite-math-only) speedup vs default build (-O3 -ffp-contract=off), Aligned64: | Kernel | M1 Max | Ryzen 1800X | |-------------------|---------------------------------|----------------------------------| | Sum (n >= 251) | 7.2-8.5x (1.1x at n=37) | 2.0x (1.8x at 4M, 1.5x at n=37) | | Dot (n >= 251) | 4.5-6.3x (1.7x at n=37) | 2.0x (1.5x at 4M, 1.9x at n=37) | | Cubic (in cache) | 1.26-1.38x | 1.41-1.48x | | Axpy | 1.0-1.07x | 1.0-1.08x | | Stencil | 1.0x | 0.89-0.97x (slower at mid sizes) | std::assume_aligned<64>: no effect on the M1. On the Ryzen it helps Sum in the native build (-27% at n=37, -11% at 251, -8% at 4M: GCC drops the alignment prologue), but the hinted Axpy is 30-35% slower in the default build (n 37-1003). EnergyPlus never used such hints (ASSUME_ALIGNED_OBJEXXFCL is unused), so OBJEXXFCL_ALIGN=64 never got that benefit. Conclusions: - Going from ObjexxFCL's 64-byte alignment to std::vector's 16 bytes costs nothing measurable on either machine, in default or native builds. Alignment is not a reason to keep ObjexxFCL arrays. - Only misalignment within an element (Offset8) costs, more so on Zen 1 AVX2, but no allocator produces that for double. - Native + fast math gains depend heavily on the CPU (reductions 7-8x on M1, 2x on Zen 1 which splits 256-bit ops in two) and on size (little at tens of elements, which is typical of EnergyPlus arrays). Supports keeping NatLabRockies/EnergyPlus#11816 opt-in. Excluded outliers (single block, not reproduced in the other build): M1 default Stencil Aligned128 4M +146% (buffer placement, Aligned64/Default have the same alignment at that size), M1 default Axpy Aligned64 4M reference 15% slow (cv 5.5%; AxpyAssumeAligned64 on the same layout was normal).
|
@jmarrec thanks. The goal here was to allow and evaluate (not implement or use) these compiler optimizations, and this PR does a good job of laying what we would be up against. The code changes here are what are required to turn them on with one new compiler flag ( I ran the ctest suite locally with OFF tests show no failures, as expected. ON tests show 2 failures on Linux 24.04, GCC 13.3.0. and 12 failures on macOS 26.7.1, Apple clang 21.0.0. Correct me if needed, but I think these are likely expected at this point given that the compiler optimizations may yield slightly different results, and thus controls can trip off/on at different points compared to before, as noted here. Addressing those would require the larger team discussion and results review I mentioned, which was not the goal here. |
You won't. The binary can't be portable because it assumes a specific CPU architecture. You have to compile E+ with native optimizations on the machine it will run on. This would be helpful for someone setting up a server that aims to run a ton of simulations (or the enthusiastic user who wouldn't mind a few percentage points). As far as getting better performance even than currently with native optimizations, it's possible but that would require rewriting loops, hinting the compiler about cache alignment, and maybe even hand rolled simd intrinsic/ assembly, which I don't think anyone is considering here. (I will touch on the topic soon because ObjexxFCL is 64 aligned currently and EPVector isn't) I'll try to reproduce the test failures and address them. |
Thanks for clarifying. This is never going into our broadly distributed production code then. |
| if(ENABLE_NATIVE_OPTIMIZATION) | ||
| # Binaries built with -march=native / -ffast-math are not portable: make it obvious so such a package never gets distributed | ||
| set(CPACK_PACKAGE_FILE_NAME "${CPACK_PACKAGE_FILE_NAME}-Native-NonPortable") | ||
| message(AUTHOR_WARNING "ENABLE_NATIVE_OPTIMIZATION is ON: the package is NOT portable and must not be distributed") | ||
| endif() | ||
|
|
There was a problem hiding this comment.
Make it crystal clear when ENABLE_NATIVE_OPTIMIZATIONS is on that you shouldn't distribute it
| // The insulation layer and the basement wall interface partitions are adjacent: derive both from their shared edge so it | ||
| // is computed only once, otherwise round-off can make them overlap | ||
| Real64 const SideXInsulationWallEdge = this->PerimeterOffset - InterfaceCellWidth; | ||
| // Side X direction - Insulation layer | ||
| SideXLocation = this->PerimeterOffset - InterfaceCellWidth - CellWidth / 2.0; | ||
| SideXLocation = SideXInsulationWallEdge - CellWidth / 2.0; | ||
| // Side X direction - Basement Wall Interface | ||
| SideXWallLocation = this->PerimeterOffset - InterfaceCellWidth / 2.0; | ||
| SideXWallLocation = SideXInsulationWallEdge + InterfaceCellWidth / 2.0; |
There was a problem hiding this comment.
The insulation layer and interface partitions share an edge but it was computed via two different expressions, so under -ffast-math they could end up 1 ULP apart and trip the overlap check (ZoneCoupledGroundHTBasement, PipingSystemFullSimulation). Compute the shared edge once and derive both partition centers from it.
|
|
|
Thanks @jmarrec. I've confirmed that the latest changes result in no ctest failures on Linux or Mac with ENABLE_NATIVE_OPTIMIZATION=ON |
|
While I'm always supportive of performance improvements, I have some concerns with this approach. Here are my comments, in no particular order, except for maybe the last one:
|
-ffast-math is a bundle of several independent flags:
```markdown
| Flag | What it does |
|------|-------------|
| `-fassociative-math` | allows reordering FP ops — this is what unlocks loop vectorization |
| `-ffp-contract=fast` | enables FMA (so it's a superset of `-ffp-contract=fast`) |
| `-ffinite-math-only` | assumes no NaN/Inf exist |
| `-fno-signed-zeros` | treats `-0 == +0` |
| `-fno-trapping-math` | assumes FP exceptions never trap |
| `-fno-math-errno` | doesn't set `errno` after math calls |
| `-freciprocal-math` | replaces `x/y` with `x*(1/y)` |
```
The dangerous ones for a simulation code are -ffinite-math-only (EnergyPlus definitely produces NaNs as error signals in some paths).
This assumes I don't care about numerical stability in regression tests
I ran a sample a of integration tests 5 times and took the mean, for a build with no march=native and one with march=native (same machine, same flags otherwise, same SSD). I have a Ryzen 1800X which is znver1 arch, so it has Zen 1's 2×128-bit AVX2, which isn't as good as Zen 3/4 where 256-bit is native or newer Intel CPUs.
Anyways, the results are a bit... unexciting. That's the issue with the way the code is written currently, I don't think it finds a bunch of loops it can vectorize...
```markdown
| | Base | Native | Delta | % |
|--------------------------------------------------------------------|--------|----------|---------|--------|
| integration.HospitalLowEnergy | 34.61 | 33.63 | -0.98 | -2.91% |
| integration.5ZoneAirCooledWithCoupledInGradeSlab | 8.10 | 7.70 | -0.40 | -5.22% |
| integration.RefBldgLargeOfficeNew2004_Chicago | 4.79 | 4.72 | -0.08 | -1.60% |
| integration.HeatPumpWaterHeaterStratified | 3.60 | 3.38 | -0.21 | -6.26% |
| integration.DOASDualDuctSchool | 2.92 | 2.80 | -0.12 | -4.28% |
| integration.FourPipeBeamLargeOffice | 2.92 | 2.88 | -0.04 | -1.30% |
| integration.RadLoTempElecTermReheatCondFD | 2.65 | 2.69 | 0.04 | +1.65% |
| integration.CoolingTower_MerkelVariableSpeed | 2.31 | 2.28 | -0.03 | -1.32% |
| integration.ChillerPartLoadCurve_RefBldgLargeOfficeNew2004_Chicago | 2.31 | 2.27 | -0.04 | -1.61% |
| integration.ConvectionAdaptiveSmallOffice | 2.08 | 2.08 | 0.01 | +0.29% |
| integration.RefBldgMediumOfficeNew2004_Chicago | 1.71 | 1.69 | -0.02 | -1.12% |
| integration.AirflowNetwork_MultiZone_SmallOffice | 1.63 | 1.61 | -0.02 | -1.23% |
| integration.RefBldgSmallOfficeNew2004_Chicago | 1.25 | 1.23 | -0.03 | -2.08% |
| integration.GSHP-GLHE | 1.17 | 1.16 | -0.01 | -0.88% |
| integration.5ZoneAirCooled | 0.88 | 0.87 | -0.01 | -1.51% |
| integration.1ZoneUncontrolled | 0.62 | 0.61 | -0.00 | -0.44% |
| Total | 73.53 | 71.59 | -1.93 | -2.70% |
```
The changes in DaylightingManager are because GCC reports uninitialized
```
/home/julien/Software/Others/EnergyPlus/src/EnergyPlus/DaylightingManager.cc: In function ‘void EnergyPlus::Dayltg::DayltgInterReflectedIllum(EnergyPlus::EnergyPlusData&, int, int, int, int)’:
/home/julien/Software/Others/EnergyPlus/src/EnergyPlus/DaylightingManager.cc:7232:12: error: ‘rfd3’ is used uninitialized [-Werror=uninitialized]
7232 | Real64 rfd3;
| ^~~~
/home/julien/Software/Others/EnergyPlus/src/EnergyPlus/DaylightingManager.cc:7229:12: error: ‘rbd1’ is used uninitialized [-Werror=uninitialized]
7229 | Real64 rbd1; // Beam-diffuse back vis reflectance of bare glass layers 1 and 2
```
./Products/energyplus --help ```diff EnergyPlus, Version 26.2.0-13b6af3090, YMD=2026.09.24 10:36 PythonLinkage: Linked to Python Version: "3.12.2 (main, Sep 9 2024, 14:39:25) [GCC 13.2.0]" -Built on Platform: Ubuntu24.04_x86_64 +Built on Platform: Ubuntu24.04_x86_64 With Native Optimizations Usage: ./Products/energyplus [OPTIONS] [input_file] [SUBCOMMAND] ```
…it tests These asserted bit-exact equality on floating point values, which breaks under -ffast-math (FMA contraction, reassociation, reciprocal approximation) since it no longer guarantees IEEE-754 bit reproducibility. Switching to the 4-ULP tolerant macros keeps the tests meaningful without hardcoding a tolerance.
…its test The unformatted JSON path prints the shortest round-trip repr of the raw double, so it picks up trailing digits when `-ffast-math` nudges the underlying value by a few ULPs (eg. "0.93768" vs "0.9376800000000001"). Parse and compare with a tolerance instead of asserting exact string equality.
…l approximation Speed-bucket matching truncated a ratio to int and compared against 100/75/50/25, relying on x/x == 1.0 exactly. Under -freciprocal-math that division can land at 0.999999..., truncating to 99 instead of 100, leaving speedsForA empty and segfaulting on the next indexed access. Round instead of truncate.
… shouldn't distribute it
…titions The insulation layer and interface partitions share an edge but it was computed via two different expressions, so under -ffast-math they could end up 1 ULP apart and trip the overlap check (ZoneCoupledGroundHTBasement, PipingSystemFullSimulation). Compute the shared edge once and derive both partition centers from it.
|
|
|
Thanks @jasondegraw. These are all fair points that I doubt anyone will broadly disagree with. As was pointed out on the iteration call on Wednesday, we already expose the PerformancePrecisionTradeoffs object that users can tweak at their will that may change their results. Obviously both do different things, but the effect is similar. A user wanting to test these compiler flags would really have to want to since they would have to compile it themselves. As you pointed, out the uncertainty around results without a side-by-side comparison should be a deterrent, and the non-portability likely would be as well. I'm personally OK with this since it doesn't change anything regarding our current builds, and also for when this same question comes up again we can point people at the PR and CMAKE flag, and say, "we tried that, several times, but go ahead and give it a shot," "we'll believe it when we see it, " and "bring your receipts and be prepared to prove it." I'll hold this open for few more workdays for any further discussion. |
Pull request overview
Description of the purpose of this PR
Add a new cmake option
ENABLE_NATIVE_OPTIMIZATION(off by default, no change to default builds)march=native(-mcpu=nativeon ARM), to benefit from compiler hardware-specific optimization-ffast-mathand-fno-finite-math-onlyon GCC/Clang, and the matching Intel/MSVC native-arch flags. Some of the options that ffast-math enables are required so the compiler is allowed to optimize things (more on this later)I fixed some unit tests that relied on exact floating point equality (EXPECT_EQ instead of EXPECT_DOUBLE_EQ), and fixed one thing in StandardRatings.cc that I'll explain shortly
-frecord-gcc-switches(GCC/Clang only , not AppleClang) to the energyplus exe (and only this one). It's an unnoticeable change but it allows you to inspect what flags where used to produce the binary. For eg, on my machine (Ryzen 1800X)regular build
when using ENABLE_NATIVE_OPTIMIZATIONS
Pull Request Author
Reviewer