fix NaN mixture log_prob gradient at zero weights - #2224
Conversation
|
Hi @esennesh, thank you. Do you have an MRE where this bug was appearing? |
The below should trigger it. That's minimal, of course. My actual use-case was a Gaussian mixture model in which the means and weights were dynamically determined by upstream latent variables, so that sometimes zero weights would occur in some components and sometimes not. Since we're working in Jax, the dynamic control flow to just leave out all zero-weight components is a bit finicky, if it's possible. |
|
@esennesh, thanks for the MRE. I have modified it a little. import jax
import jax.numpy as jnp
import numpyro.distributions as dist
def log_prob(probs):
mixing_distribution = dist.Categorical(probs=probs, validate_args=True)
component_distributions = dist.Normal(
loc=jnp.array([0.0, 1.0, 2.0]),
scale=1.0,
validate_args=True,
)
return dist.MixtureSameFamily(
mixing_distribution,
component_distributions,
validate_args=True,
).log_prob(0.3)
log_prob_jit = jax.jit(log_prob)
mixing_probs = jnp.array([0.0, 0.5, 0.5])
log_prob_val = log_prob(mixing_probs)
print(log_prob_val)
log_prob_val = log_prob_jit(mixing_probs)
print(log_prob_val)
with jax.debug_nans(True):
grad_log_prob_val = jax.grad(log_prob)(mixing_probs)
print(grad_log_prob_val)
with jax.debug_nans(True):
grad_log_prob_val = jax.grad(log_prob_jit)(mixing_probs)
print(grad_log_prob_val)It is reproducing the same error. This is the output before fix: and after fix: I assume the numerical answers are correct. And the fix I have proposed is, def _to_logits_multinom(probs: ArrayLike) -> ArrayLike:
- minval = jnp.finfo(jnp.result_type(probs)).min
- return jnp.clip(jnp.log(probs), minval)
+ safe_probs = jnp.where(probs > 0, probs, 1.0)
+ safe_log_probs = jnp.where(probs > 0, jnp.log(safe_probs), -jnp.inf)
+ return safe_log_probs@juanitorduz what are your thoughts? |
|
Do I understand correctly that the issue can be addressed with simpler change? |
Yes. The MRE does pass with the little change. Although I have not tested it on the rest of the test suite. |
|
Hi @esennesh, have you had a chance to try my fix in your workflow to see if it is working? |
|
Doing that tonight or tomorrow morning, most likely.
…On Sat, Aug 1, 2026, 6:52 AM Meesum Qazalbash ***@***.***> wrote:
*Qazalbash* left a comment (pyro-ppl/numpyro#2224)
<#2224 (comment)>
Hi @esennesh <https://github.com/esennesh>, have you had a chance to try
my fix in your workflow to see if it is working?
—
Reply to this email directly, view it on GitHub
<#2224?email_source=notifications&email_token=AAFRWFNCDBPQL2S5WL4ZVA35HXYYJA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMJVGE3TCMJQGU32M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5151711057>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFRWFMD6J6HP4GT5LANXHD5HXYYJAVCNFSNUABFKJSXA33TNF2G64TZHMYTOMBVHAYDKNBQHNEXG43VMU5TIOJTGQZDMNBTHAZ2C5QC>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/AAFRWFMDOTK5OR4AUMEHEFD5HXYYJA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMJVGE3TCMJQGU32M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y>
and Android
<https://github.com/notifications/mobile/android/AAFRWFPCHQPKH4RWXICNZAD5HXYYJA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMJVGE3TCMJQGU32M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>.
Download it today!
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
a3eca3a to
0c03ec4
Compare
|
Ok, plainly I need to re-fix the regex and assertions in the tests to handle the unwrapped case. I'll fold that into the second patch and force-push again and then we should have a minimal PR for which all the checks pass. |
Benchmark reportthis PR run time: unchanged across 32 benchmarks
- compile time: 1 slower, 0 fasterSignificant changes (1) ──────── run time ─────── ────── compile time ──────
benchmark baseline this PR Δ baseline this PR Δ
────────────────────────────────────────────────────────────────────────────────────────
- predictive_forward_sampling 725.9 ms 718.3 ms -1.1% 160.9 ms 213.9 ms +32.9%Red is slower, green is faster; a row is coloured by the worse of its two columns. A delta in parentheses cleared the threshold on a measurement below the resolution floor, so it is shown without being called a change. † marks a benchmark that could not be compared — see below. Full results
|
| baseline | this PR | |
|---|---|---|
| ref | master |
bugfix/null_mixture_weights_grad |
| commit | 1f6877a0 |
2e891eda |
| numpyro | 0.21.0 | 0.21.0 |
| jax | 0.11.0 | 0.11.0 |
| backend | cpu | cpu |
| python | 3.14.7 | 3.14.7 |
Runner: Linux-6.17.0-1020-azure-x86_64-with-glibc2.39, 4 CPUs.
Produced by this benchmark run.
Signed-off-by: Eli Sennesh <elisennesh@astera.org>
…ights Testing Done: ??? Signed-off-by: Eli Sennesh <elisennesh@astera.org>
Testing Done: pre-commit formatting checks pass, as does modified test Signed-off-by: Eli Sennesh <elisennesh@gmail.com>
b7a8701 to
2e891ed
Compare
|
I've corrected the patches on this branch and rebased atop |
|
Oh weird. Now the checks all pass. Neat! Two reviews requested from @juanitorduz and @Qazalbash , for what's now a much more minimal patch. |
_MixtureBase.log_probwrote the density in log-weight form vialog_softmax(mixing.logits) + log p_k. For a probs-parameterized mixingCategoricalwith an exact-zero weight, logits takes log(0): the forward value was masked to finite but the VJP evaluated 1/0 = inf, producing a NaN gradient into every parameter feeding the weights.