Skip to content

Thomas/re enable tests#63

Merged
gallantandre merged 28 commits into
mainfrom
Thomas/Re-enable-tests
Jun 14, 2026
Merged

Thomas/re enable tests#63
gallantandre merged 28 commits into
mainfrom
Thomas/Re-enable-tests

Conversation

@Nikos-d

@Nikos-d Nikos-d commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@Nikos-d Nikos-d marked this pull request as ready for review June 4, 2026 15:08
random_int used a shared static std::mt19937, which is a data race when
guesses are generated in parallel via Taskflow (mt19937::operator()
mutates state). Switch rd/gen to static thread_local to match
get_random() above; each thread seeds its own generator once. The
distribution stays local so the [min, max] range can vary per call.
Clearer naming alongside random_int. Updates the forward declaration in
blast_math.hpp, the definition in math/misc.hpp, and all call sites in
math/Matrix.hpp, math/Array.hpp, and the gpu/ trajectory and manipulator
headers. get_random_guesses is a separate function and is unchanged.
The dev preset hardcoded CMAKE_CXX_FLAGS=-march=native and relied on a
 condition to gate per OS. That conflates OS with
compiler: on Windows the flag depends on the toolchain (cl/clang-cl want
/arch:AVX2; MinGW GCC and Clang's GNU driver want -march=native), and a
preset condition cannot inspect the detected compiler.

The arch flag is already applied correctly per-target as PRIVATE in
examples/ and tests/ (/arch:AVX2 for MSVC, -march=native otherwise), so
the global preset flag was a redundant, broken duplicate. Drop it and the
OS conditions, and merge dev-msvc into a single dev preset that is correct
for MSVC, GCC, Clang and MinGW on any OS. Add configuration:RelWithDebInfo
to the dev build preset so multi-config generators (Visual Studio) build
the intended config instead of the default.
Blast's headers call AVX2/FMA intrinsics directly (e.g. _mm256_fmadd_pd
in math/Array.hpp), but the arch flags were never propagated to consumers
and there was no guard, so find_package(Blast) users hit a cryptic
'target specific option mismatch' on GCC/Clang (or silent degradation on
MSVC). The CI smoke test only built a Vec3, so it never caught this.

- blast/blast: add an #error guard that reports a clear, per-compiler
  message when AVX2 (and FMA, off MSVC) is missing. CUDA builds opt out.
- blast/CMakeLists.txt: propagate -mavx2 -mfma as INTERFACE for GCC/Clang.
  Feature flags are additive, so they never downgrade a consumer's
  -march=native. MSVC is excluded (its /arch: is mutually exclusive and
  would downgrade an AVX512 consumer with D9025); it relies on the guard.
- CI: the consumer smoke test now calls blast::dot, exercising the AVX2/FMA
  path with no flags set, so it verifies the propagated contract.
- CLAUDE.md: document the requirement and correct the stale claim that
  blast/CMakeLists.txt sets these flags.
Resolves the modify/delete conflict on tests/benchmarks/bench_functions.cpp
by keeping the deletion: the file references test_helper/ which was removed
as part of this branch.
@gallantandre gallantandre merged commit ad1c8c6 into main Jun 14, 2026
5 checks passed
@gallantandre gallantandre deleted the Thomas/Re-enable-tests branch June 14, 2026 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants