Open
Conversation
Ozaq
force-pushed
the
fix-issues
branch
2 times, most recently
from
68d3a70 to
26c5336
Compare
schroedtert
reviewed
Feb 17, 2026
Contributor
schroedtert
left a comment
schroedtert
left a comment
There was a problem hiding this comment.
First quick review, need to check the visitor change for the model formatting and pipeline changes in more detail.
| F_rep.x, | ||
| F_rep.y, | ||
| K_ij); | ||
| throw std::runtime_error( |
Contributor
There was a problem hiding this comment.
Do we really want to stop the simulation in that case? Different behavior than before. We had that discussion in the past already iirc
| @@ -179,12 +179,12 @@ def desired_direction(self, desired_direction): | |||
| @deprecated("deprecated, use 'desired_direction' instead.") | |||
Contributor
There was a problem hiding this comment.
Shouldn't this then also state desired_orientation?
| return self._obj.desired_orientation | ||
|
|
||
| @e0.setter | ||
| @deprecated("deprecated, use 'desired_direction' instead.") |
Contributor
There was a problem hiding this comment.
Shouldn't this then also state desired_orientation?
| strategy: | ||
| matrix: | ||
| os: [ubuntu-latest, windows-latest, macos-14, macos-14, macos-15, macos-26] | ||
| os: [ubuntu-latest, windows-latest, macos-14, macos-15] |
Contributor
There was a problem hiding this comment.
From that I can see macos-26 is a valid image, although it is still in beta state. We could already use it here. (edit 26-02-28 no longer beta)
From https://github.com/actions/runner-images?tab=readme-ov-file#available-images:
The holes() lambda builds the result vector but never returns it, causing Python callers to always receive None. This breaks Geometry.as_wkt() and any code relying on hole extraction. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The overloaded visitor only handles 3 of 5 variant types, and the
SocialForceModelData handler is placed outside the overloaded{}
braces. Replace with a generic lambda that covers all variant types
uniformly, since the format string is identical for each.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
*kwargs unpacks the dict keys as positional arguments instead of forwarding keyword arguments. Use **kwargs so that excluded_areas reaches build_geometry() as intended. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
pybind11 converts Python None to nullptr for raw pointer args. The lambda dereferences model without checking, causing a segfault. Add a null check and throw std::invalid_argument instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ForceRepPed can produce NaN from division by zero when maxNeighborInteractionDistance equals maxNeighborInterpolationDistance, making dist_intpol_right zero. The same applies to geometry parameters in ForceInterpolation. Add parameter validation in the builder to reject degenerate combinations (interaction distance must exceed interpolation distance). Replace the silent NaN log-and-return with a throw to catch regressions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
setup.py checked for CMake >= 3.19 but CMakeLists.txt requires >= 3.22. Users with CMake 3.19-3.21 passed the Python check but hit a confusing FATAL_ERROR from cmake_minimum_required. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The deprecated e0 getter/setter on GeneralizedCentrifugalForceModelState incorrectly accessed desired_speed instead of desired_orientation, returning a float where a direction tuple was expected and silently overwriting the wrong field on set. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The tests job used system-default compilers which drift silently as runner images update. Pin to explicit versions (gcc-14 via ubuntu-toolchain-r PPA, clang-21 via apt.llvm.org) so warnings from new compiler releases surface immediately. Fix the checks job LLVM repo from jammy to noble to match ubuntu-latest. Fix the test-wheels OS matrix which had a duplicate macos-14 entry and an invalid macos-26. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The existing BUILD_WITH_ASAN option only covered Clang and only enabled AddressSanitizer. Replace it with BUILD_WITH_SANITIZERS which enables both ASAN and UBSAN for Clang and GCC. Add a dedicated CI job that builds with sanitizers and runs both C++ unit tests and Python system tests. The Python tests need LD_PRELOAD of the shared ASAN runtime, PYTHONMALLOC=malloc to bypass pymalloc, and detect_leaks=0 to suppress CPython allocator noise. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Preview: https://PedestrianDynamics.github.io/jupedsim/pull-requests/1528/