analysis: raise instead of NaN for empty particle types#5372
Merged
jngrad merged 3 commits intoJun 30, 2026
Merged
Conversation
check_particle_type only verifies 0 <= p_type <= max_seen_particle_type, and max_seen is never decremented when particles are removed. A seen-then-deleted type (or any populated-then-emptied type) therefore passes validation while contributing an empty buffer / zero total mass. On such a type the analysis routines divided by zero: - center_of_mass returned com/mass = 0/0 = NaN (the `double mass = 1.` placeholder was a red herring: boost::mpi::reduce overwrites the root's output with the reduced sum, so it never prevented the division). - gyration_tensor divided by buf_pos.size() == 0; its non-empty precondition was only a release-stripped assert(), so it produced NaN in Release and aborted in RelWithAssert. moment_of_inertia_matrix inherits center_of_mass, so it threw too. Fix (PREVENT, matching the subsystem convention where bad analysis preconditions throw): - center_of_mass: all_reduce the total mass to every rank and throw a std::runtime_error when it is zero. The throw is collective so the script interface does not deadlock on the following MPI collective. - gyration_tensor: replace the assert with a real throw on rank 0. - Analysis.cpp: move the core calls and their MPI collectives inside context()->parallel_try_catch (mirroring calc_rg) so the new throws are marshalled correctly across MPI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tics-empty-type-div0
jngrad
marked this pull request as ready for review
June 30, 2026 15:41
jngrad
approved these changes
Jun 30, 2026
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
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.
check_particle_type only verifies 0 <= p_type <= max_seen_particle_type,
and max_seen is never decremented when particles are removed. A
seen-then-deleted type (or any populated-then-emptied type) therefore
passes validation while contributing an empty buffer / zero total mass.
On such a type the analysis routines divided by zero:
double mass = 1.placeholder was a red herring: boost::mpi::reduce overwrites the root's
output with the reduced sum, so it never prevented the division).
precondition was only a release-stripped assert(), so it produced NaN
in Release and aborted in RelWithAssert.
moment_of_inertia_matrix inherits center_of_mass, so it threw too.
Fix (PREVENT, matching the subsystem convention where bad analysis
preconditions throw):
std::runtime_error when it is zero. The throw is collective so the
script interface does not deadlock on the following MPI collective.
context()->parallel_try_catch (mirroring calc_rg) so the new throws
are marshalled correctly across MPI.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
🤖 Generated with Claude Code