Raw predicted log_mass in decode_secondaries can crash log_transform during rollout #54

New Issue

2026-08-14T13:52:56+02:00

lars commented

2026-08-14 13:52:56 +02:00

decode_secondaries (giant/data/transforms.py:684-690) applies inv_log_transform() directly to the model's raw predicted log_mass:

log_mass = sec_cont[:, :, 4]  # (N, K)
...
sec_mass = np.where(sec_valid, inv_log_transform(log_mass), 0.0).astype(np.float32)

log_mass isn't itself the output of log_transform, so it isn't guaranteed to land in a range that round-trips cleanly:

Too negative and exp(log_mass) undershoots _EPS, making inv_log_transform(log_mass) = exp(log_mass) - _EPS go slightly negative.
Too positive and exp(log_mass) overflows float32 to inf.

Either one crashes the next rollout step: once that secondary spawns a track, its mass is fed back in as conditioning (giant/rollout.py → build_cond_features → _physical_cond_columns), and log_transform(mass) computes log(mass + eps) — non-finite for mass <= -eps or mass = inf.

Proposal: clip log_mass to [log(_EPS), _LOG_MASS_MAX] before inverting, guaranteeing a finite, non-negative mass — matching the invariant the surrounding comment already assumed but didn't enforce. _LOG_MASS_MAX should sit comfortably below float32's overflow point (~88.7) while being far beyond any physical particle mass a converged model would predict (80.0 was used previously).

A fix for this (clip + regression tests for both tails) was already implemented on fix/rollout-negative-secondary-mass, but that branch forked from a stale master and predates several since-merged changes to transforms.py/network.py (e.g. gitea #35, #36), so it can't be merged as-is. Reimplement the same fix against current master.

`decode_secondaries` (`giant/data/transforms.py:684-690`) applies `inv_log_transform()` directly to the model's raw predicted `log_mass`: ```python log_mass = sec_cont[:, :, 4] # (N, K) ... sec_mass = np.where(sec_valid, inv_log_transform(log_mass), 0.0).astype(np.float32) ``` `log_mass` isn't itself the output of `log_transform`, so it isn't guaranteed to land in a range that round-trips cleanly: - Too negative and `exp(log_mass)` undershoots `_EPS`, making `inv_log_transform(log_mass) = exp(log_mass) - _EPS` go slightly **negative**. - Too positive and `exp(log_mass)` **overflows** float32 to `inf`. Either one crashes the *next* rollout step: once that secondary spawns a track, its mass is fed back in as conditioning (`giant/rollout.py` → `build_cond_features` → `_physical_cond_columns`), and `log_transform(mass)` computes `log(mass + eps)` — non-finite for `mass <= -eps` or `mass = inf`. **Proposal:** clip `log_mass` to `[log(_EPS), _LOG_MASS_MAX]` before inverting, guaranteeing a finite, non-negative mass — matching the invariant the surrounding comment already assumed but didn't enforce. `_LOG_MASS_MAX` should sit comfortably below float32's overflow point (~88.7) while being far beyond any physical particle mass a converged model would predict (80.0 was used previously). -------- A fix for this (clip + regression tests for both tails) was already implemented on `fix/rollout-negative-secondary-mass`, but that branch forked from a stale master and predates several since-merged changes to `transforms.py`/`network.py` (e.g. gitea #35, #36), so it can't be merged as-is. Reimplement the same fix against current master.

lars added the bug label 2026-08-14 13:52:56 +02:00

lars commented

2026-08-17 09:31:43 +02:00

Fixed in bacc876 on fix/issue-54.

decode_secondaries now clips the raw predicted log_mass to [log(_EPS), _LOG_MASS_MAX=80.0] before inv_log_transform, guaranteeing a finite, non-negative mass — matching the invariant the surrounding comment already assumed but didn't enforce. Added regression tests for both the extreme-negative and extreme-positive tails (asserting the decoded mass round-trips through log_transform without raising, which is the actual downstream crash path). Reimplemented fresh against current master rather than merging the stale fix/rollout-negative-secondary-mass branch, since that branch forked before #35/#36; the stale branch was left untouched per the user's call. No follow-up filed.

Fixed in bacc876 on fix/issue-54. decode_secondaries now clips the raw predicted log_mass to [log(_EPS), _LOG_MASS_MAX=80.0] before inv_log_transform, guaranteeing a finite, non-negative mass — matching the invariant the surrounding comment already assumed but didn't enforce. Added regression tests for both the extreme-negative and extreme-positive tails (asserting the decoded mass round-trips through log_transform without raising, which is the actual downstream crash path). Reimplemented fresh against current master rather than merging the stale fix/rollout-negative-secondary-mass branch, since that branch forked before #35/#36; the stale branch was left untouched per the user's call. No follow-up filed.

lars closed this issue

2026-08-17 09:31:44 +02:00

lars referenced this issue from a commit

2026-08-17 09:34:01 +02:00

Clip raw predicted log_mass in decode_secondaries (gitea #54)

lars referenced this issue

2026-08-17 09:34:06 +02:00

Clip raw predicted log_mass in decode_secondaries (gitea #54) #62

lars referenced this issue from a commit

2026-08-17 09:42:56 +02:00

Merge pull request 'Clip raw predicted log_mass in decode_secondaries (gitea #54)' (#62) from fix/issue-54 into master

Sign in to join this conversation.