Skip to content

Commit 469cc89

Browse files
ptheywoodptheywood
committed
Fix: Improve wrapped spatial messaging bounds/radius compatibility checking
Improves checking that the communication radius is a factor of the envionment dimensions when using spaical communication with seatbelts enabled This was preivously not always correct due to floating point use for some common cases, including 1 / 0.05f. This is now checked once on the host, and stored in the messageing MetaData so that if the wrapped communicator is used on device a runtime exception can be raised. A new internal (detail) header is creatd with a function that is common to 2D and 3D implementations. Closes #1177
1 parent d8fa00a commit 469cc89

10 files changed

Lines changed: 85 additions & 11 deletions

File tree

‎include/flamegpu/detail/numeric.h‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
#ifndef INCLUDE_FLAMEGPU_DETAIL_NUMERIC_H_
2+
#define INCLUDE_FLAMEGPU_DETAIL_NUMERIC_H_
3+
4+
#include <algorithm>
5+
#include <limits>
6+
#include <cmath>
7+
8+
namespace flamegpu {
9+
namespace detail {
10+
/**
11+
* Internal (detail) methods related to numbers.
12+
*/
13+
namespace numeric {
14+
15+
/**
16+
* Templated method to check whether a value (x) is approximately exactly divisible by another value (y).
17+
*
18+
* @param x the numerator
19+
* @param y the denominator
20+
*
21+
* @return boolean indicating if x is exactly divisible by y, within reason given the use of floating point numbers.
22+
*/
23+
template <typename T>
24+
bool approxExactlyDivisible(T x, T y) {
25+
// Scale machine epsilon by the magnitude of the larger value
26+
T scaledEpsilon = std::max(std::abs(x), std::abs(y)) * std::numeric_limits<T>::epsilon();
27+
// Compute the remainder
28+
T v = std::fmod(x, y);
29+
// approx equal if the remainder is within scaledEpsilon of 0 or b (fmod(1, 0.05f) returns ~0.05f)
30+
return v <= scaledEpsilon || v > y - scaledEpsilon;
31+
}
32+
33+
} // namespace numeric
34+
} // namespace detail
35+
} // namespace flamegpu
36+
37+
#endif // INCLUDE_FLAMEGPU_DETAIL_NUMERIC_H_

‎include/flamegpu/runtime/messaging/MessageSpatial2D.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ class MessageSpatial2D {
6262
* max-lowerBound
6363
*/
6464
float environmentWidth[2];
65+
/**
66+
* If the environment width is exactly divisible by the radius in all dimensions, allowing wrapped access.
67+
*/
68+
bool wrapCompatible;
6569
};
6670
};
6771

‎include/flamegpu/runtime/messaging/MessageSpatial2D/MessageSpatial2DDevice.cuh‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -500,15 +500,14 @@ class MessageSpatial2D::In {
500500
// Return iterator at min corner of env, this should be safe
501501
return WrapFilter(metadata, metadata->min[0], metadata->min[1]);
502502
}
503-
if (fmodf(metadata->max[0] - metadata->min[0], metadata->radius) > 0.00001f ||
504-
fmodf(metadata->max[1] - metadata->min[1], metadata->radius) > 0.00001f) {
503+
if (!metadata->wrapCompatible) {
505504
DTHROW("Spatial messaging radius (%g) is not a factor of environment dimensions (%g, %g),"
506505
" this is unsupported for the wrapped iterator, MessageSpatial2D::In::wrap().\n", metadata->radius,
507-
metadata->max[0] - metadata->min[0],
508-
metadata->max[1] - metadata->min[1]);
506+
metadata->environmentWidth[0],
507+
metadata->environmentWidth[1]);
509508
}
510509
#endif
511-
return WrapFilter(metadata, x, y);
510+
return WrapFilter(metadata, x, y);
512511
}
513512

514513
/**

‎include/flamegpu/runtime/messaging/MessageSpatial3D.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ class MessageSpatial3D {
6161
* max-lowerBound
6262
*/
6363
float environmentWidth[3];
64+
/**
65+
* If the environment width is exactly divisible by the radius in all dimensions, allowing wrapped access.
66+
*/
67+
bool wrapCompatible;
6468
};
6569
};
6670

‎include/flamegpu/runtime/messaging/MessageSpatial3D/MessageSpatial3DDevice.cuh‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -539,14 +539,12 @@ class MessageSpatial3D::In {
539539
// Return iterator at min corner of env, this should be safe
540540
return WrapFilter(metadata, metadata->min[0], metadata->min[1], metadata->min[2]);
541541
}
542-
if (fmodf(metadata->max[0] - metadata->min[0], metadata->radius) > 0.00001f ||
543-
fmodf(metadata->max[1] - metadata->min[1], metadata->radius) > 0.00001f ||
544-
fmodf(metadata->max[2] - metadata->min[2], metadata->radius) > 0.00001f) {
542+
if (!metadata->wrapCompatible) {
545543
DTHROW("Spatial messaging radius (%g) is not a factor of environment dimensions (%g, %g, %g),"
546544
" this is unsupported for the wrapped iterator, MessageSpatial3D::In::wrap().\n", metadata->radius,
547-
metadata->max[0] - metadata->min[0],
548-
metadata->max[1] - metadata->min[1],
549-
metadata->max[2] - metadata->min[2]);
545+
metadata->environmentWidth[0],
546+
metadata->environmentWidth[1],
547+
metadata->environmentWidth[2]);
550548
}
551549
#endif
552550
return WrapFilter(metadata, x, y, z);

‎src/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@ SET(SRC_INCLUDE
292292
${FLAMEGPU_ROOT}/include/flamegpu/detail/CUDAEventTimer.cuh
293293
${FLAMEGPU_ROOT}/include/flamegpu/detail/cuda.cuh
294294
${FLAMEGPU_ROOT}/include/flamegpu/detail/cxxname.hpp
295+
${FLAMEGPU_ROOT}/include/flamegpu/detail/numeric.h
295296
${FLAMEGPU_ROOT}/include/flamegpu/detail/SignalHandlers.h
296297
${FLAMEGPU_ROOT}/include/flamegpu/detail/StaticAssert.h
297298
${FLAMEGPU_ROOT}/include/flamegpu/detail/SteadyClockTimer.h

‎src/flamegpu/runtime/messaging/MessageSpatial2D.cu‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "flamegpu/simulation/detail/CUDAScatter.cuh"
3131
#include "flamegpu/util/nvtx.h"
3232
#include "flamegpu/detail/cuda.cuh"
33+
#include "flamegpu/detail/numeric.h"
3334

3435
namespace flamegpu {
3536
MessageSpatial2D::CUDAModelHandler::CUDAModelHandler(detail::CUDAMessage &a)
@@ -42,11 +43,13 @@ MessageSpatial2D::CUDAModelHandler::CUDAModelHandler(detail::CUDAMessage &a)
4243
hd_data.min[1] = d.minY;
4344
hd_data.max[0] = d.maxX;
4445
hd_data.max[1] = d.maxY;
46+
hd_data.wrapCompatible = true;
4547
binCount = 1;
4648
for (unsigned int axis = 0; axis < 2; ++axis) {
4749
hd_data.environmentWidth[axis] = hd_data.max[axis] - hd_data.min[axis];
4850
hd_data.gridDim[axis] = static_cast<unsigned int>(ceil(hd_data.environmentWidth[axis] / hd_data.radius));
4951
binCount *= hd_data.gridDim[axis];
52+
hd_data.wrapCompatible = hd_data.wrapCompatible && flamegpu::detail::numeric::approxExactlyDivisible<float>(hd_data.environmentWidth[axis], hd_data.radius);
5053
}
5154
}
5255
MessageSpatial2D::CUDAModelHandler::~CUDAModelHandler() { }

‎src/flamegpu/runtime/messaging/MessageSpatial3D.cu‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "flamegpu/runtime/messaging/MessageSpatial3D/MessageSpatial3DHost.h"
66
#include "flamegpu/runtime/messaging/MessageSpatial3D/MessageSpatial3DDevice.cuh"
77
#include "flamegpu/detail/cuda.cuh"
8+
#include "flamegpu/detail/numeric.h"
89
#include "flamegpu/simulation/detail/CUDAScatter.cuh"
910

1011
#ifdef _MSC_VER
@@ -39,11 +40,13 @@ MessageSpatial3D::CUDAModelHandler::CUDAModelHandler(detail::CUDAMessage &a)
3940
hd_data.max[0] = d.maxX;
4041
hd_data.max[1] = d.maxY;
4142
hd_data.max[2] = d.maxZ;
43+
hd_data.wrapCompatible = true;
4244
binCount = 1;
4345
for (unsigned int axis = 0; axis < 3; ++axis) {
4446
hd_data.environmentWidth[axis] = hd_data.max[axis] - hd_data.min[axis];
4547
hd_data.gridDim[axis] = static_cast<unsigned int>(ceil(hd_data.environmentWidth[axis] / hd_data.radius));
4648
binCount *= hd_data.gridDim[axis];
49+
hd_data.wrapCompatible = hd_data.wrapCompatible && flamegpu::detail::numeric::approxExactlyDivisible<float>(hd_data.environmentWidth[axis], hd_data.radius);
4750
}
4851
// Device allocation occurs in allocateMetaDataDevicePtr rather than the constructor.
4952
}

‎tests/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ SET(TESTS_SRC
1717
${CMAKE_CURRENT_SOURCE_DIR}/test_cases/detail/test_wddm.cu
1818
${CMAKE_CURRENT_SOURCE_DIR}/test_cases/detail/test_dependency_versions.cu
1919
${CMAKE_CURRENT_SOURCE_DIR}/test_cases/detail/test_multi_thread_device.cu
20+
${CMAKE_CURRENT_SOURCE_DIR}/test_cases/detail/test_numeric.cpp
2021
${CMAKE_CURRENT_SOURCE_DIR}/test_cases/detail/test_CUDAEventTimer.cu
2122
${CMAKE_CURRENT_SOURCE_DIR}/test_cases/detail/test_SteadyClockTimer.cpp
2223
${CMAKE_CURRENT_SOURCE_DIR}/test_cases/detail/test_cxxname.cpp

‎tests/test_cases/detail/test_numeric.cpp‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
#include <string>
2+
#include "flamegpu/detail/numeric.h"
3+
4+
#include "gtest/gtest.h"
5+
6+
namespace flamegpu {
7+
8+
TEST(TestNumeric, approxExactlyDivisible) {
9+
/**
10+
* Test that approxExactlyDivisible returns the expected value(s) for a number of cases, including values which were an issue for the original wrapped spatial messaging implementation (see #1157)
11+
*/
12+
ASSERT_TRUE(detail::numeric::approxExactlyDivisible<float>(1, 0.05f));
13+
ASSERT_TRUE(detail::numeric::approxExactlyDivisible<float>(1, 0.04f));
14+
ASSERT_TRUE(detail::numeric::approxExactlyDivisible<float>(2, 0.05f));
15+
ASSERT_TRUE(detail::numeric::approxExactlyDivisible<float>(1, 0.005f));
16+
ASSERT_TRUE(detail::numeric::approxExactlyDivisible<float>(50.0f, 10));
17+
ASSERT_TRUE(detail::numeric::approxExactlyDivisible<float>(100000, 0.05f));
18+
19+
ASSERT_FALSE(detail::numeric::approxExactlyDivisible<float>(1, 0.03f));
20+
ASSERT_FALSE(detail::numeric::approxExactlyDivisible<float>(50.1f, 10));
21+
ASSERT_FALSE(detail::numeric::approxExactlyDivisible<float>(100000, 0.03f));
22+
}
23+
24+
} // namespace flamegpu

0 commit comments

Comments
 (0)