Skip to content

Fix Spatial Wrapped invalid env/radii detection#1182

Merged
mondus merged 4 commits into
masterfrom
spatial-radius-factor-fix
Sep 2, 2025
Merged

Fix Spatial Wrapped invalid env/radii detection#1182
mondus merged 4 commits into
masterfrom
spatial-radius-factor-fix

Conversation

@ptheywood
Copy link
Copy Markdown
Member

@ptheywood ptheywood commented Feb 2, 2024

  • Reproduce python-native edge case in c++ test suite
  • Expand edge case tests coverage to exposes a broader range of floating point issues
  • Fix tests
  • Optional: Migrate float logic to the host, making in-device checks a single global read.
  • Update the example back to using a radius of 0.05

Closes #1177

@ptheywood
Copy link
Copy Markdown
Member Author

The following appears to work for values tested so far. Not the cheapest method so worth only doing this once on the host and storing if wrapped is available for a given spatial message list on the device, rather than doing this per call to the wrapped iterator on the device

template <typename T>
bool approxExactlyDivisible(T a, T b) {
    // Scale machine epsilon by the magnitude of the larger value
    T scaledEpsilon = std::max(std::abs(a), std::abs(b)) * std::numeric_limits<T>::epsilon();
    // Compute the remainder
    T v = std::fmod(a, b);
    // approx equal if the remainder is within scaledEpsilon of 0 or b (fmod(1, 0.05f) returns ~0.05f)
    return v <= scaledEpsilon || v > b - scaledEpsilon;
}

@ptheywood ptheywood mentioned this pull request Nov 19, 2024
7 tasks
@ptheywood ptheywood force-pushed the spatial-radius-factor-fix branch from 7751622 to 29eb33d Compare August 19, 2025 11:05
@ptheywood ptheywood force-pushed the spatial-radius-factor-fix branch from 29eb33d to 12ad320 Compare September 1, 2025 16:35
@ptheywood ptheywood requested a review from mondus September 1, 2025 16:35
@ptheywood ptheywood marked this pull request as ready for review September 1, 2025 16:36
@ptheywood ptheywood force-pushed the spatial-radius-factor-fix branch from 12ad320 to 8395a67 Compare September 1, 2025 16:40
@ptheywood ptheywood changed the base branch from master to ci-manylinux-setuptools-wheel September 1, 2025 16:40
SEtuptoolls and wheels were removed from the base image for all pythons (not just < 3.12) on 2025-06-22.
@ptheywood ptheywood force-pushed the ci-manylinux-setuptools-wheel branch from 6dfb069 to d0c21b0 Compare September 1, 2025 16:41
@ptheywood ptheywood force-pushed the spatial-radius-factor-fix branch from 8395a67 to 53e860f Compare September 1, 2025 16:42
…ecking

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
@ptheywood ptheywood force-pushed the spatial-radius-factor-fix branch from 53e860f to 360b6d3 Compare September 1, 2025 17:10
Base automatically changed from ci-manylinux-setuptools-wheel to master September 2, 2025 10:04
@mondus mondus merged commit b21fc79 into master Sep 2, 2025
26 checks passed
@mondus mondus deleted the spatial-radius-factor-fix branch September 2, 2025 10:07
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.

Wrapped MsgSpatial interaction radius factor bug

2 participants