analysis: accept p_type=-1 for center_of_mass and angular_momentum (bug-sweep #50)#5373
Draft
RudolfWeeber wants to merge 1 commit into
Draft
Conversation
…ug-sweep #50) The script-interface handlers for center_of_mass and angular_momentum unconditionally called check_particle_type(p_type), which throws for any p_type < 0. This rejected the documented and core-supported p_type == -1 sentinel ("all non-virtual particles"), so system.analysis.angular_momentum (p_type=-1) and center_of_mass(p_type=-1) raised "Particle type -1 does not exist" instead of computing the sum over all particles. The core functions center_of_mass and angular_momentum already implement the -1 case ((p.type() == p_type or p_type == -1)). Guard the validation so the documented sentinel bypasses check_particle_type while explicit types are still validated. The scope is intentionally limited to these two methods: min_dist and distribution still (correctly) reject -1. Also align the center_of_mass docstring with angular_momentum's already-correct text mentioning the -1 sentinel, and add regression tests asserting the -1 result equals the sum over all non-virtual particles (distinct from the type-0-only result, so a wrong-subset regression is also caught). Co-Authored-By: Claude Opus 4.8 <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
1 participant
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.
The script-interface handlers for center_of_mass and angular_momentum
unconditionally called check_particle_type(p_type), which throws for any
p_type < 0. This rejected the documented and core-supported p_type == -1
sentinel ("all non-virtual particles"), so system.analysis.angular_momentum
(p_type=-1) and center_of_mass(p_type=-1) raised "Particle type -1 does not
exist" instead of computing the sum over all particles.
The core functions center_of_mass and angular_momentum already implement the
-1 case ((p.type() == p_type or p_type == -1)). Guard the validation so the
documented sentinel bypasses check_particle_type while explicit types are
still validated. The scope is intentionally limited to these two methods:
min_dist and distribution still (correctly) reject -1.
Also align the center_of_mass docstring with angular_momentum's already-correct
text mentioning the -1 sentinel, and add regression tests asserting the -1
result equals the sum over all non-virtual particles (distinct from the
type-0-only result, so a wrong-subset regression is also caught).
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
🤖 Generated with Claude Code