Clamp analysis histogram bins before the i32 cast, not after (gitea #61) #63

Merged

lars merged 2 commits from fix/issue-61 into master 2026-08-17 10:17:33 +02:00

Conversation 0 Commits 2 Files Changed 2

+38 -2

lars commented 2026-08-17 09:43:25 +02:00

Owner

Copy Link

Copy Source

_bin_expr in giant/analysis/reduce.py clipped the bin index to
[0, nbins-1] only after casting it to Int32, so the clip never got a
chance to run: a rollout step_length of 1.0725e10 mm against fixed
edges [2.9e-5, 94.04] with 50 bins produces a raw index of ~5.7e9,
which overflows i32 and fails the strict cast, killing the whole
compute-one job. Same failure mode for +/-inf.

Clamp in f64 first, then cast to Int32. NaN has no edge to clamp to,
so it maps to null and is dropped in the two callers (hist1d,
profile_partial) — matching what np.histogram does with NaN, and what
profile_partial needs anyway since a null bin index would break its
np.add.at.

This reimplements commit 313373c, which fixed the same bug but landed
on a branch (fix/rollout-negative-secondary-mass) that forked off a
stale master and was never merged; reduce.py has since diverged enough
that the original diff no longer applies cleanly.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com

_bin_expr in giant/analysis/reduce.py clipped the bin index to [0, nbins-1] only after casting it to Int32, so the clip never got a chance to run: a rollout step_length of 1.0725e10 mm against fixed edges [2.9e-5, 94.04] with 50 bins produces a raw index of ~5.7e9, which overflows i32 and fails the strict cast, killing the whole compute-one job. Same failure mode for +/-inf. Clamp in f64 first, then cast to Int32. NaN has no edge to clamp to, so it maps to null and is dropped in the two callers (hist1d, profile_partial) — matching what np.histogram does with NaN, and what profile_partial needs anyway since a null bin index would break its np.add.at. This reimplements commit 313373c, which fixed the same bug but landed on a branch (fix/rollout-negative-secondary-mass) that forked off a stale master and was never merged; reduce.py has since diverged enough that the original diff no longer applies cleanly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

lars added 1 commit 2026-08-17 09:43:33 +02:00

Clamp analysis histogram bins before the i32 cast, not after (gitea #61) ... a746efb6e1

_bin_expr in giant/analysis/reduce.py clipped the bin index to
[0, nbins-1] only after casting it to Int32, so the clip never got a
chance to run: a rollout step_length of 1.0725e10 mm against fixed
edges [2.9e-5, 94.04] with 50 bins produces a raw index of ~5.7e9,
which overflows i32 and fails the strict cast, killing the whole
compute-one job. Same failure mode for +/-inf.

Clamp in f64 first, then cast to Int32. NaN has no edge to clamp to,
so it maps to null and is dropped in the two callers (hist1d,
profile_partial) — matching what np.histogram does with NaN, and what
profile_partial needs anyway since a null bin index would break its
np.add.at.

This reimplements commit 313373c, which fixed the same bug but landed
on a branch (fix/rollout-negative-secondary-mass) that forked off a
stale master and was never merged; reduce.py has since diverged enough
that the original diff no longer applies cleanly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

lars added 1 commit 2026-08-17 09:51:39 +02:00

Merge branch 'master' into fix/issue-61 867a07da2b

lars merged commit 7bf0bea56a into master 2026-08-17 10:17:33 +02:00

lars deleted branch fix/issue-61 2026-08-17 10:17:33 +02:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

architecture

bug

Silent/wrong behavior, no crash

experiment

feature

New feature matching the v0.3 redesign

investigation

modularity

Making the network genuinely pluggable

performance

phase2

validation

wontfix

Not being fixed right now

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

lars (Lars Bogner)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: lars/giant#63