Skip to content

Commit ad1c8c6

Browse files
Nikos-dThomasDuquettegallantandre
authored
Massive changes to enable a bunch of tests (#63)
* Uncommented some tests * uncommented & fixed test_objmatrix * Moved rpy <-> rotation matrix test to misc test * Cleaned up manipulators + uncommented Gen3 test * Started UR5e test but bug was found * Fixed UR5e bug from merge * Few cleanups/fixes * style: apply clang-format * Added/modified tests * style: apply clang-format * removed test_helper - moved to utililites - moved scenes.hpp to blast/world/ * style: apply clang-format * Fix + add manip constructor test * style: apply clang-format * utilities include fix * Fix in adding payload + tool * minor change * proposed fix for missing dll in tests * Make random_int thread-safe with thread_local generator 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. * Rename get_random to random_real 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. * Collapse dev/dev-msvc into one cross-platform preset 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. * Make AVX2+FMA requirement explicit for consumers 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. --------- Co-authored-by: ThomasDuquette <133025159+ThomasDuquette@users.noreply.github.com> Co-authored-by: Andre Gallant <gallantandre@gmail.com>
1 parent 4ca5e5a commit ad1c8c6

67 files changed

Lines changed: 2809 additions & 6549 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/workflows/cmake-multi-platform.yml‎

Lines changed: 17 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -15,44 +15,25 @@ jobs:
1515
strategy:
1616
fail-fast: false
1717
matrix:
18-
os: [ubuntu-latest, windows-latest]
19-
build_type: [Release]
20-
c_compiler: [gcc, clang, cl]
2118
include:
22-
- os: windows-latest
23-
c_compiler: cl
24-
cpp_compiler: cl
2519
- os: ubuntu-latest
26-
c_compiler: gcc
27-
cpp_compiler: g++
20+
preset: ci-gcc
2821
- os: ubuntu-latest
29-
c_compiler: clang
30-
cpp_compiler: clang++
31-
exclude:
32-
- os: windows-latest
33-
c_compiler: gcc
22+
preset: ci-clang
3423
- os: windows-latest
35-
c_compiler: clang
36-
- os: ubuntu-latest
37-
c_compiler: cl
24+
preset: ci
3825

3926
steps:
4027
- uses: actions/checkout@v4
4128

4229
- name: Configure CMake
43-
run: >
44-
cmake -B ${{ github.workspace }}/build
45-
-DCMAKE_CXX_COMPILER=${{ matrix.cpp_compiler }}
46-
-DCMAKE_C_COMPILER=${{ matrix.c_compiler }}
47-
-DCMAKE_BUILD_TYPE=${{ matrix.build_type }}
48-
-S ${{ github.workspace }}
30+
run: cmake --preset ${{ matrix.preset }}
4931

5032
- name: Build
51-
run: cmake --build ${{ github.workspace }}/build --config ${{ matrix.build_type }}
33+
run: cmake --build --preset ${{ matrix.preset }}
5234

5335
- name: Test
54-
working-directory: ${{ github.workspace }}/build
55-
run: ctest --build-config ${{ matrix.build_type }} --output-on-failure
36+
run: ctest --preset ${{ matrix.preset }}
5637

5738
# -------------------------------------------------------------------------
5839
# Install + consumer smoke test
@@ -92,17 +73,21 @@ jobs:
9273
target_link_libraries(test_consumer PRIVATE Blast::blast)
9374
EOF
9475
cat > "$RUNNER_TEMP/consumer/main.cpp" <<'EOF'
95-
// Minimal consumer — verifies that the installed Blast headers and the
96-
// exported nlopt target are all reachable via find_package(Blast).
76+
// Consumer smoke test — verifies that find_package(Blast) exposes the
77+
// headers/nlopt AND that the AVX2+FMA contract holds: blast::dot uses
78+
// _mm256_fmadd_pd, so this fails to compile unless the installed
79+
// Blast::blast target propagates the required ISA flags (or the
80+
// #error guard fires). No flags are set here on purpose — that is what
81+
// the test checks.
9782
#include <blast>
9883
#include <iostream>
9984
10085
int main() {
101-
// Exercise a type from blast_math.hpp (included transitively via <blast>)
102-
blast::Vec3 v{1.0, 2.0, 3.0};
103-
std::cout << "Blast::Vec3 constructed: "
104-
<< v.x << " " << v.y << " " << v.z << "\n";
105-
return 0;
86+
blast::Array a({1.0, 2.0, 3.0, 4.0});
87+
blast::Array b({4.0, 3.0, 2.0, 1.0});
88+
blast::real d = blast::dot(a, b); // exercises the AVX2/FMA path
89+
std::cout << "Blast::dot = " << d << "\n";
90+
return (d == 20.0) ? 0 : 1; // 1*4 + 2*3 + 3*2 + 4*1 = 20
10691
}
10792
EOF
10893

‎CMakePresets.json‎

Lines changed: 66 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,62 +2,101 @@
22
"version": 6,
33
"configurePresets": [
44
{
5-
"name": "dev",
6-
"displayName": "Developer build (Linux/macOS)",
7-
"description": "RelWithDebInfo, native tuning, compile-commands for tooling",
8-
"binaryDir": "${sourceDir}/build/${presetName}",
9-
"condition": {
10-
"type": "notEquals",
11-
"lhs": "${hostSystemName}",
12-
"rhs": "Windows"
13-
},
5+
"name": "base",
6+
"hidden": true,
147
"cacheVariables": {
15-
"CMAKE_BUILD_TYPE": "RelWithDebInfo",
16-
"CMAKE_CXX_FLAGS": "-march=native",
17-
"CMAKE_EXPORT_COMPILE_COMMANDS": "ON"
8+
"BUILD_SHARED_LIBS": "OFF"
189
}
1910
},
2011
{
21-
"name": "dev-msvc",
22-
"displayName": "Developer build (Windows/MSVC)",
23-
"description": "RelWithDebInfo with AVX2, compile-commands for tooling",
12+
"name": "dev",
13+
"inherits": "base",
14+
"displayName": "Developer build",
15+
"description": "RelWithDebInfo with compile-commands for tooling. AVX2/native tuning is applied per dev target (see examples/ and tests/, which pick /arch:AVX2 for MSVC and -march=native otherwise), so this preset is correct for MSVC, GCC, Clang and MinGW on any OS.",
2416
"binaryDir": "${sourceDir}/build/${presetName}",
25-
"condition": {
26-
"type": "equals",
27-
"lhs": "${hostSystemName}",
28-
"rhs": "Windows"
29-
},
3017
"cacheVariables": {
3118
"CMAKE_BUILD_TYPE": "RelWithDebInfo",
32-
"CMAKE_CXX_FLAGS": "/arch:AVX2",
3319
"CMAKE_EXPORT_COMPILE_COMMANDS": "ON"
3420
}
3521
},
3622
{
3723
"name": "ci",
38-
"displayName": "CI build",
24+
"inherits": "base",
25+
"displayName": "CI build (MSVC/default)",
3926
"description": "Release build without machine-specific tuning",
4027
"binaryDir": "${sourceDir}/build/${presetName}",
4128
"cacheVariables": {
4229
"CMAKE_BUILD_TYPE": "Release",
4330
"CMAKE_EXPORT_COMPILE_COMMANDS": "ON"
4431
}
32+
},
33+
{
34+
"name": "ci-gcc",
35+
"inherits": "ci",
36+
"displayName": "CI build (GCC)",
37+
"binaryDir": "${sourceDir}/build/${presetName}",
38+
"cacheVariables": {
39+
"CMAKE_C_COMPILER": "gcc",
40+
"CMAKE_CXX_COMPILER": "g++"
41+
}
42+
},
43+
{
44+
"name": "ci-clang",
45+
"inherits": "ci",
46+
"displayName": "CI build (Clang)",
47+
"binaryDir": "${sourceDir}/build/${presetName}",
48+
"cacheVariables": {
49+
"CMAKE_C_COMPILER": "clang",
50+
"CMAKE_CXX_COMPILER": "clang++"
51+
}
4552
}
4653
],
4754
"buildPresets": [
4855
{
4956
"name": "dev",
50-
"configurePreset": "dev"
51-
},
52-
{
53-
"name": "dev-msvc",
54-
"configurePreset": "dev-msvc",
57+
"configurePreset": "dev",
5558
"configuration": "RelWithDebInfo"
5659
},
5760
{
5861
"name": "ci",
5962
"configurePreset": "ci",
6063
"configuration": "Release"
64+
},
65+
{
66+
"name": "ci-gcc",
67+
"configurePreset": "ci-gcc",
68+
"configuration": "Release"
69+
},
70+
{
71+
"name": "ci-clang",
72+
"configurePreset": "ci-clang",
73+
"configuration": "Release"
74+
}
75+
],
76+
"testPresets": [
77+
{
78+
"name": "ci",
79+
"configurePreset": "ci",
80+
"configuration": "Release",
81+
"output": {
82+
"outputOnFailure": true
83+
}
84+
},
85+
{
86+
"name": "ci-gcc",
87+
"configurePreset": "ci-gcc",
88+
"configuration": "Release",
89+
"output": {
90+
"outputOnFailure": true
91+
}
92+
},
93+
{
94+
"name": "ci-clang",
95+
"configurePreset": "ci-clang",
96+
"configuration": "Release",
97+
"output": {
98+
"outputOnFailure": true
99+
}
61100
}
62101
]
63102
}

‎blast/CMakeLists.txt‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,22 @@ if(ENABLE_TRACY)
7272
target_link_libraries(blast INTERFACE TracyClient)
7373
endif()
7474

75-
# -march=native / /arch:AVX2 are machine-specific developer flags.
76-
# They must NOT be INTERFACE — that would force them on every downstream
77-
# consumer, breaking cross-compilation and reproducible builds.
78-
# Add them to your personal build via CMakePresets.json (see root preset).
75+
# -march=native is machine-specific *tuning*. It must NOT be INTERFACE — that
76+
# would force it on every downstream consumer, breaking cross-compilation and
77+
# reproducible builds. The dev targets (examples/, tests/) apply it per-target
78+
# as PRIVATE, choosing /arch:AVX2 for MSVC and -march=native otherwise.
79+
#
80+
# AVX2+FMA, by contrast, is a hard *minimum-ISA requirement*: Blast's headers
81+
# call AVX2/FMA intrinsics directly (e.g. _mm256_fmadd_pd), so consumers cannot
82+
# compile without them. Propagate the feature flags so find_package(Blast)
83+
# consumers just work. Feature flags are additive — they never downgrade a
84+
# consumer's -march=native. MSVC is intentionally excluded: its /arch: is a
85+
# single mutually-exclusive level, so forcing /arch:AVX2 would downgrade an
86+
# /arch:AVX512 consumer (warning D9025); MSVC is covered by the #error guard in
87+
# blast/blast instead.
88+
target_compile_options(blast INTERFACE
89+
$<$<CXX_COMPILER_ID:GNU,Clang>:-mavx2;-mfma>
90+
)
7991

8092
# --------------------------------------------------------------------------
8193
# Install rules

‎blast/blast‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,27 @@ using i64 = int64_t;
4545
#define device_fn
4646
#endif
4747

48+
// ---------------------------------------------------------------------------
49+
// Instruction-set requirement
50+
// Blast's math headers use AVX2+FMA intrinsics directly (e.g. _mm256_fmadd_pd
51+
// in math/Array.hpp). Without those features enabled, GCC/Clang fail with a
52+
// cryptic "target specific option mismatch" and MSVC silently degrades. Fail
53+
// early here with an actionable message instead. CUDA builds opt out.
54+
// ---------------------------------------------------------------------------
55+
#if !defined(__NVCC__) && !defined(__CUDACC__)
56+
#if defined(_MSC_VER) && !defined(__clang__)
57+
// MSVC: /arch:AVX2 (and /arch:AVX512) define __AVX2__ and imply FMA.
58+
#if !defined(__AVX2__)
59+
#error "Blast requires AVX2. Compile with /arch:AVX2 (or higher)."
60+
#endif
61+
#else
62+
// GCC, Clang, clang-cl: both macros are set when the features are enabled.
63+
#if !defined(__AVX2__) || !defined(__FMA__)
64+
#error "Blast requires AVX2 and FMA. Compile with -mavx2 -mfma (or -march=native)."
65+
#endif
66+
#endif
67+
#endif
68+
4869
#define VCL_NAMESPACE vcl
4970
#include "vectorclass.h"
5071
#include "vectormath_trig.h"
@@ -74,3 +95,7 @@ using i64 = int64_t;
7495
#include "blast_manipulator.hpp"
7596
#include "blast_optimization.hpp"
7697
#include "blast_trajectory.hpp"
98+
99+
#include "utilities/file_io.hpp"
100+
#include "utilities/is_close.hpp"
101+
#include "utilities/print.hpp"

‎blast/blast_math.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ inline blast_fn real wrap2pi(real);
353353
inline blast_fn real wrap_to_180(real);
354354
inline blast_fn real deg2rad(real);
355355
inline blast_fn real rad2deg(real);
356-
inline blast_fn real get_random();
356+
inline blast_fn real random_real();
357357
inline blast_fn real clamp(real val, real mini, real maxi);
358358
inline blast_fn real& clamp_inplace(real& val, real mini, real maxi);
359359
inline blast_fn real sign(real v);

0 commit comments

Comments
 (0)