Fix Spatial Wrapped invalid env/radii detection

.github/workflows/Draft-Release.yml

@@ -414,17 +414,12 @@ jobs:
         make install
 
     - name: Install setuptools and wheel
-      if: ${{ env.PYTHON >= 3.12 }}
       run: |
         python${{env.PYTHON}} -m pip install setuptools wheel
+
     - name: Add custom problem matchers for annotations
       run: echo "::add-matcher::.github/problem-matchers.json"
 
-    - name: Install setuptools and wheel
-      if: ${{ env.PYTHON >= 3.12 }}
-      run: |
-        python${{env.PYTHON}} -m pip install setuptools wheel
-
     # This patches a bug where ManyLinux doesn't generate buildnumber as git dir is owned by diff user
     - name: Enable git safe-directory
       run: git config --global --add safe.directory $GITHUB_WORKSPACE

.github/workflows/Manylinux_2_28.yml

@@ -134,7 +134,6 @@ jobs:
         make install
 
     - name: Install setuptools and wheel
-      if: ${{ env.PYTHON >= 3.12 }}
       run: |
         python${{env.PYTHON}} -m pip install setuptools wheel
 

README.md

@@ -374,4 +374,3 @@ For a full list of known issues pleases see the [Issue Tracker](https://github.c
 + Warnings and a loss of performance due to hash collisions in device code ([#356](https://github.com/FLAMEGPU/FLAMEGPU2/issues/356))
 + Multiple known areas where performance can be improved (e.g. [#449](https://github.com/FLAMEGPU/FLAMEGPU2/issues/449), [#402](https://github.com/FLAMEGPU/FLAMEGPU2/issues/402))
 + CUDA 12.2+ suffers from poor RTC compilation times, to be fixed in a future release ([#1118](https://github.com/FLAMEGPU/FLAMEGPU2/issues/1118)).
-+ Wrapped spatial message iteration with may incorrectly report that the radius is not a factor of the environment with `FLAMEGPU_SEATBELTS=ON` for certain floating point values, to be fixed in a future release ([#1177](https://github.com/FLAMEGPU/FLAMEGPU2/issues/1177)).

examples/python_native/boids_spatial3D_wrapped/boids_spatial3D.py

@@ -292,7 +292,7 @@ def inputdata(message_in: pyflamegpu.MessageSpatial3D, message_out: pyflamegpu.M
 env.newPropertyFloat("MIN_INITIAL_SPEED", 0.1)
 
 # Interaction radius
-env.newPropertyFloat("INTERACTION_RADIUS", 0.04)  # 0.04 is a workaround for wrapped spatial messaging bug
+env.newPropertyFloat("INTERACTION_RADIUS", 0.05)
 env.newPropertyFloat("SEPARATION_RADIUS", 0.01)
 
 # Global Scalers

include/flamegpu/detail/numeric.h

@@ -0,0 +1,37 @@
+#ifndef INCLUDE_FLAMEGPU_DETAIL_NUMERIC_H_
+#define INCLUDE_FLAMEGPU_DETAIL_NUMERIC_H_
+
+#include <algorithm>
+#include <limits>
+#include <cmath>
+
+namespace flamegpu {
+namespace detail {
+/**
+ * Internal (detail) methods related to numbers.
+ */
+namespace numeric {
+
+/**
+ * Templated method to check whether a value (x) is approximately exactly divisible by another value (y).
+ * 
+ * @param x the numerator
+ * @param y the denominator
+ * 
+ * @return boolean indicating if x is exactly divisible by y, within reason given the use of floating point numbers.
+ */
+template <typename T>
+bool approxExactlyDivisible(T x, T y) {
+    // Scale machine epsilon by the magnitude of the larger value
+    T scaledEpsilon = std::max(std::abs(x), std::abs(y)) * std::numeric_limits<T>::epsilon();
+    // Compute the remainder
+    T v = std::fmod(x, y);
+    // approx equal if the remainder is within scaledEpsilon of 0 or b (fmod(1, 0.05f) returns ~0.05f)
+    return v <= scaledEpsilon || v > y - scaledEpsilon;
+}
+
+}  // namespace numeric
+}  // namespace detail
+}  // namespace flamegpu
+
+#endif  // INCLUDE_FLAMEGPU_DETAIL_NUMERIC_H_

include/flamegpu/runtime/messaging/MessageSpatial2D.h

@@ -62,6 +62,10 @@ class MessageSpatial2D {
          * max-lowerBound
          */
         float environmentWidth[2];
+        /**
+         * If the environment width is exactly divisible by the radius in all dimensions, allowing wrapped access.
+         */
+        bool wrapCompatible;
     };
 };
 

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

@@ -500,15 +500,14 @@ class MessageSpatial2D::In {
              // Return iterator at min corner of env, this should be safe
              return WrapFilter(metadata, metadata->min[0], metadata->min[1]);
          }
-         if (fmodf(metadata->max[0] - metadata->min[0], metadata->radius) > 0.00001f ||
-             fmodf(metadata->max[1] - metadata->min[1], metadata->radius) > 0.00001f) {
+         if (!metadata->wrapCompatible) {
              DTHROW("Spatial messaging radius (%g) is not a factor of environment dimensions (%g, %g),"
                  " this is unsupported for the wrapped iterator, MessageSpatial2D::In::wrap().\n", metadata->radius,
-                 metadata->max[0] - metadata->min[0],
-                 metadata->max[1] - metadata->min[1]);
+                 metadata->environmentWidth[0],
+                 metadata->environmentWidth[1]);
          }
 #endif
-         return WrapFilter(metadata, x, y);
+        return WrapFilter(metadata, x, y);
      }
 
     /**

include/flamegpu/runtime/messaging/MessageSpatial3D.h

@@ -61,6 +61,10 @@ class MessageSpatial3D {
          * max-lowerBound
          */
         float environmentWidth[3];
+        /**
+         * If the environment width is exactly divisible by the radius in all dimensions, allowing wrapped access.
+         */
+        bool wrapCompatible;
     };
 };
 

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

@@ -539,14 +539,12 @@ class MessageSpatial3D::In {
             // Return iterator at min corner of env, this should be safe
             return WrapFilter(metadata, metadata->min[0], metadata->min[1], metadata->min[2]);
         }
-        if (fmodf(metadata->max[0] - metadata->min[0], metadata->radius) > 0.00001f ||
-            fmodf(metadata->max[1] - metadata->min[1], metadata->radius) > 0.00001f ||
-            fmodf(metadata->max[2] - metadata->min[2], metadata->radius) > 0.00001f) {
+        if (!metadata->wrapCompatible) {
             DTHROW("Spatial messaging radius (%g) is not a factor of environment dimensions (%g, %g, %g),"
                 " this is unsupported for the wrapped iterator, MessageSpatial3D::In::wrap().\n", metadata->radius,
-                metadata->max[0] - metadata->min[0],
-                metadata->max[1] - metadata->min[1],
-                metadata->max[2] - metadata->min[2]);
+                metadata->environmentWidth[0],
+                metadata->environmentWidth[1],
+                metadata->environmentWidth[2]);
         }
 #endif
         return WrapFilter(metadata, x, y, z);

src/CMakeLists.txt

@@ -292,6 +292,7 @@ SET(SRC_INCLUDE
     ${FLAMEGPU_ROOT}/include/flamegpu/detail/CUDAEventTimer.cuh
     ${FLAMEGPU_ROOT}/include/flamegpu/detail/cuda.cuh
     ${FLAMEGPU_ROOT}/include/flamegpu/detail/cxxname.hpp
+    ${FLAMEGPU_ROOT}/include/flamegpu/detail/numeric.h
     ${FLAMEGPU_ROOT}/include/flamegpu/detail/SignalHandlers.h
     ${FLAMEGPU_ROOT}/include/flamegpu/detail/StaticAssert.h
     ${FLAMEGPU_ROOT}/include/flamegpu/detail/SteadyClockTimer.h

src/flamegpu/runtime/messaging/MessageSpatial2D.cu

@@ -30,6 +30,7 @@
 #include "flamegpu/simulation/detail/CUDAScatter.cuh"
 #include "flamegpu/util/nvtx.h"
 #include "flamegpu/detail/cuda.cuh"
+#include "flamegpu/detail/numeric.h"
 
 namespace flamegpu {
 MessageSpatial2D::CUDAModelHandler::CUDAModelHandler(detail::CUDAMessage &a)
@@ -42,11 +43,13 @@ MessageSpatial2D::CUDAModelHandler::CUDAModelHandler(detail::CUDAMessage &a)
     hd_data.min[1] = d.minY;
     hd_data.max[0] = d.maxX;
     hd_data.max[1] = d.maxY;
+    hd_data.wrapCompatible = true;
     binCount = 1;
     for (unsigned int axis = 0; axis < 2; ++axis) {
         hd_data.environmentWidth[axis] = hd_data.max[axis] - hd_data.min[axis];
         hd_data.gridDim[axis] = static_cast<unsigned int>(ceil(hd_data.environmentWidth[axis] / hd_data.radius));
         binCount *= hd_data.gridDim[axis];
+        hd_data.wrapCompatible = hd_data.wrapCompatible && flamegpu::detail::numeric::approxExactlyDivisible<float>(hd_data.environmentWidth[axis], hd_data.radius);
     }
 }
 MessageSpatial2D::CUDAModelHandler::~CUDAModelHandler() { }

src/flamegpu/runtime/messaging/MessageSpatial3D.cu

@@ -5,6 +5,7 @@
 #include "flamegpu/runtime/messaging/MessageSpatial3D/MessageSpatial3DHost.h"
 #include "flamegpu/runtime/messaging/MessageSpatial3D/MessageSpatial3DDevice.cuh"
 #include "flamegpu/detail/cuda.cuh"
+#include "flamegpu/detail/numeric.h"
 #include "flamegpu/simulation/detail/CUDAScatter.cuh"
 
 #ifdef _MSC_VER
@@ -39,11 +40,13 @@ MessageSpatial3D::CUDAModelHandler::CUDAModelHandler(detail::CUDAMessage &a)
     hd_data.max[0] = d.maxX;
     hd_data.max[1] = d.maxY;
     hd_data.max[2] = d.maxZ;
+    hd_data.wrapCompatible = true;
     binCount = 1;
     for (unsigned int axis = 0; axis < 3; ++axis) {
         hd_data.environmentWidth[axis] = hd_data.max[axis] - hd_data.min[axis];
         hd_data.gridDim[axis] = static_cast<unsigned int>(ceil(hd_data.environmentWidth[axis] / hd_data.radius));
         binCount *= hd_data.gridDim[axis];
+        hd_data.wrapCompatible = hd_data.wrapCompatible && flamegpu::detail::numeric::approxExactlyDivisible<float>(hd_data.environmentWidth[axis], hd_data.radius);
     }
     // Device allocation occurs in allocateMetaDataDevicePtr rather than the constructor.
 }

tests/CMakeLists.txt

@@ -17,6 +17,7 @@ SET(TESTS_SRC
     ${CMAKE_CURRENT_SOURCE_DIR}/test_cases/detail/test_wddm.cu
     ${CMAKE_CURRENT_SOURCE_DIR}/test_cases/detail/test_dependency_versions.cu
     ${CMAKE_CURRENT_SOURCE_DIR}/test_cases/detail/test_multi_thread_device.cu
+    ${CMAKE_CURRENT_SOURCE_DIR}/test_cases/detail/test_numeric.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/test_cases/detail/test_CUDAEventTimer.cu
     ${CMAKE_CURRENT_SOURCE_DIR}/test_cases/detail/test_SteadyClockTimer.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/test_cases/detail/test_cxxname.cpp

tests/test_cases/detail/test_numeric.cpp

@@ -0,0 +1,24 @@
+#include <string>
+#include "flamegpu/detail/numeric.h"
+
+#include "gtest/gtest.h"
+
+namespace flamegpu {
+
+TEST(TestNumeric, approxExactlyDivisible) {
+    /**
+     * 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) 
+     */
+    ASSERT_TRUE(detail::numeric::approxExactlyDivisible<float>(1, 0.05f));
+    ASSERT_TRUE(detail::numeric::approxExactlyDivisible<float>(1, 0.04f));
+    ASSERT_TRUE(detail::numeric::approxExactlyDivisible<float>(2, 0.05f));
+    ASSERT_TRUE(detail::numeric::approxExactlyDivisible<float>(1, 0.005f));
+    ASSERT_TRUE(detail::numeric::approxExactlyDivisible<float>(50.0f, 10));
+    ASSERT_TRUE(detail::numeric::approxExactlyDivisible<float>(100000, 0.05f));
+
+    ASSERT_FALSE(detail::numeric::approxExactlyDivisible<float>(1, 0.03f));
+    ASSERT_FALSE(detail::numeric::approxExactlyDivisible<float>(50.1f, 10));
+    ASSERT_FALSE(detail::numeric::approxExactlyDivisible<float>(100000, 0.03f));
+}
+
+}  // namespace flamegpu

tests/test_cases/runtime/messaging/test_spatial_2d.cu

@@ -4,15 +4,13 @@
 * Tests cover:
 * > mandatory messaging, send/recieve
 */
+#include <array>
 #include <unordered_map>
-
 #include "flamegpu/flamegpu.h"
-
 #include "gtest/gtest.h"
 
 namespace flamegpu {
 
-
 namespace test_message_spatial2d {
 
 FLAMEGPU_AGENT_FUNCTION(out_mandatory2D, MessageNone, MessageSpatial2D) {
@@ -918,15 +916,12 @@ FLAMEGPU_AGENT_FUNCTION(in_wrapped_EnvDimsNotFactor, MessageSpatial2D, MessageNo
     }
     return ALIVE;
 }
-TEST(Spatial2DMessageTest, Wrapped_EnvDimsNotFactor) {
-    // This tests that bug #1157 is fixed
-    // When the interaction radius is not a factor of the width
-    // that agent's near the max env bound all have the full interaction radius
+void wrapped_2d_test_bounds(std::array<float, 2> lower, std::array<float, 2> upper, float radius, bool exceptionExpected) {
     ModelDescription m("model");
     MessageSpatial2D::Description message = m.newMessage<MessageSpatial2D>("location");
-    message.setMin(0, 0);
-    message.setMax(50.1f, 50.1f);
-    message.setRadius(10);
+    message.setMin(lower.at(0), lower.at(1));
+    message.setMax(upper.at(0), upper.at(1));
+    message.setRadius(radius);
     message.newVariable<flamegpu::id_t>("id");  // unused by current test
     AgentDescription agent = m.newAgent("agent");
     agent.newVariable<float>("x");
@@ -947,11 +942,28 @@ TEST(Spatial2DMessageTest, Wrapped_EnvDimsNotFactor) {
     // Vertical pair that can interact
     // Top side
     AgentVector::Agent i1 = population[0];
-    i1.setVariable<float>("x", 25.0f);
-    i1.setVariable<float>("y", 25.0f);
+    i1.setVariable<float>("x", upper.at(0));
+    i1.setVariable<float>("y", upper.at(1));
     c.setPopulationData(population);
     c.SimulationConfig().steps = 1;
-    EXPECT_THROW(c.simulate(), exception::DeviceError);
+    if (exceptionExpected) {
+        EXPECT_THROW(c.simulate(), exception::DeviceError);
+    } else {
+        EXPECT_NO_THROW(c.simulate());
+    }
+}
+TEST(Spatial2DMessageTest, Wrapped_EnvDimsNotFactor) {
+    // This tests that bug #1157 is fixed
+    // When the interaction radius is not a factor of the width
+    // that agent's near the max env bound all have the full interaction radius
+    wrapped_2d_test_bounds({0, 0}, {50.1f, 50.1f}, 10, true);
+    wrapped_2d_test_bounds({0, 0}, {1, 1}, 0.03f, true);
+
+    // also includes a number of potential edge cases to ensure that no false positives are included (#1177)
+    wrapped_2d_test_bounds({0, 0}, {1, 1}, 0.05f, false);
+    wrapped_2d_test_bounds({0, 0}, {1, 1}, 0.04f, false);
+    wrapped_2d_test_bounds({0, 0}, {2, 1}, 0.05f, false);
+    wrapped_2d_test_bounds({0, 0}, {1, 1}, 0.005f, false);
 }
 #else
 TEST(Spatial2DMessageTest, DISABLED_Wrapped_OutOfBounds) { }

tests/test_cases/runtime/messaging/test_spatial_3d.cu

@@ -4,15 +4,13 @@
 * Tests cover:
 * > mandatory messaging, send/recieve
 */
+#include <array>
 #include <unordered_map>
-
 #include "flamegpu/flamegpu.h"
-
 #include "gtest/gtest.h"
 
 namespace flamegpu {
 
-
 namespace test_message_spatial3d {
 
 FLAMEGPU_AGENT_FUNCTION(out_mandatory3D, MessageNone, MessageSpatial3D) {
@@ -900,7 +898,7 @@ FLAMEGPU_AGENT_FUNCTION(inWrapped3D, MessageSpatial3D, MessageNone) {
 void wrapped_3d_test(const float x_offset, const float y_offset, const float z_offset, const float out_of_bounds = 0) {
     std::unordered_map<int, unsigned int> bin_counts;
     // Construct model
-    ModelDescription model("Spatial2DMessageTestModel");
+    ModelDescription model("Spatial3DMessageTestModel");
     {   // Location message
         MessageSpatial3D::Description message = model.newMessage<MessageSpatial3D>("location");
         message.setMin(0 + x_offset, 0 + y_offset, 0 + z_offset);
@@ -987,15 +985,12 @@ FLAMEGPU_AGENT_FUNCTION(in_wrapped_EnvDimsNotFactor, MessageSpatial3D, MessageNo
     }
     return ALIVE;
 }
-TEST(Spatial3DMessageTest, Wrapped_EnvDimsNotFactor) {
-    // This tests that bug #1157 is fixed
-    // When the interaction radius is not a factor of the width
-    // that agent's near the max env bound all have the full interaction radius
+void wrapped_3d_test_bounds(std::array<float, 3> lower, std::array<float, 3> upper, float radius, bool exceptionExpected) {
     ModelDescription m("model");
     MessageSpatial3D::Description message = m.newMessage<MessageSpatial3D>("location");
-    message.setMin(0, 0, 0);
-    message.setMax(50.1f, 50.1f, 50.1f);
-    message.setRadius(10);
+    message.setMin(lower.at(0), lower.at(1), lower.at(2));
+    message.setMax(upper.at(0), upper.at(1), upper.at(2));
+    message.setRadius(radius);
     message.newVariable<flamegpu::id_t>("id");  // unused by current test
     AgentDescription agent = m.newAgent("agent");
     agent.newVariable<float>("x");
@@ -1017,12 +1012,29 @@ TEST(Spatial3DMessageTest, Wrapped_EnvDimsNotFactor) {
     // Vertical pair that can interact
     // Top side
     AgentVector::Agent i1 = population[0];
-    i1.setVariable<float>("x", 25.0f);
-    i1.setVariable<float>("y", 25.0f);
-    i1.setVariable<float>("z", 25.0f);
+    i1.setVariable<float>("x", upper.at(0));
+    i1.setVariable<float>("y", upper.at(1));
+    i1.setVariable<float>("z", upper.at(2));
     c.setPopulationData(population);
     c.SimulationConfig().steps = 1;
-    EXPECT_THROW(c.simulate(), exception::DeviceError);
+    if (exceptionExpected) {
+        EXPECT_THROW(c.simulate(), exception::DeviceError);
+    } else {
+        EXPECT_NO_THROW(c.simulate());
+    }
+}
+TEST(Spatial3DMessageTest, Wrapped_EnvDimsNotFactor) {
+    // This tests that bug #1157 is fixed
+    // When the interaction radius is not a factor of the width
+    // that agent's near the max env bound all have the full interaction radius
+    wrapped_3d_test_bounds({0, 0, 0}, {50.1f, 50.1f, 50.1f}, 10, true);
+    wrapped_3d_test_bounds({0, 0, 0}, {1, 1, 1}, 0.03f, true);
+
+    // also includes a number of potential edge cases to ensure that no false positives are included (#1177)
+    wrapped_3d_test_bounds({0, 0, 0}, {1, 1, 1}, 0.05f, false);
+    wrapped_3d_test_bounds({0, 0, 0}, {1, 1, 1}, 0.04f, false);
+    wrapped_3d_test_bounds({0, 0, 0}, {3, 2, 1}, 0.05f, false);
+    wrapped_3d_test_bounds({0, 0, 0}, {1, 1, 1}, 0.005f, false);
 }
 #else
 TEST(Spatial3DMessageTest, DISABLED_Wrapped_OutOfBounds) { }