Skip to content

Conversation

asifzubair
Copy link
Contributor

@asifzubair asifzubair commented Jul 27, 2025

Description

Previously, calculating the log-probability (logp) of an AdvancedSubTensor that included a None index would cause the system to crash with an unhandled internal AttributeError.

This change resolves the bug by adding a guard within the logp rewriting logic in mixture.py. Instead of crashing, the system now correctly identifies the ambiguous operation and raises a controlled RuntimeError.

A new test, test_advanced_subtensor_none_and_integer, has been added to test_mixture.py to validate this fix and prevent regressions.

Reproducible example from the ticket now executes:

>>> import numpy as np
... import pymc as pm
... 
... 
... obs = np.random.default_rng().normal(size=(7, 4))
... with pm.Model():
...    inds = np.arange(obs.shape[1])
...    a = pm.Normal("a", shape=10)
...    b = pm.Deterministic("b", a[None, inds])
...    c = pm.Normal("c", mu=b, sigma=1, observed=obs)
...    pm.sample()
...    
Initializing NUTS using jitter+adapt_diag...
Multiprocess sampling (3 chains in 3 jobs)
NUTS: [a]
                                                                                                                                  
  Progress                                   Draws   Divergences   Step size   Grad evals   Sampling Speed   Elapsed   Remaining  
 ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── 
  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━   2000    0             0.871       3            792.47 draws/s   0:00:02   0:00:00    
  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━   2000    0             0.819       3            807.38 draws/s   0:00:02   0:00:00    
  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━   2000    0             0.951       3            783.45 draws/s   0:00:02   0:00:00    
                                                                                                                                  
Sampling 3 chains for 1_000 tune and 1_000 draw iterations (3_000 + 3_000 draws total) took 3 seconds.
We recommend running at least 4 chains for robust computation of convergence diagnostics
Inference data with groups:
	> posterior
	> sample_stats
	> observed_data

Related Issue

Checklist

Type of change

  • New feature / enhancement
  • Bug fix
  • Documentation
  • Maintenance
  • Other (please specify):

📚 Documentation preview 📚: https://pymc--7877.org.readthedocs.build/en/7877/

Copy link

codecov bot commented Jul 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.94%. Comparing base (58b49f2) to head (d25ce81).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #7877   +/-   ##
=======================================
  Coverage   92.94%   92.94%           
=======================================
  Files         116      116           
  Lines       18845    18845           
=======================================
  Hits        17516    17516           
  Misses       1329     1329           
Files with missing lines Coverage Δ
pymc/logprob/mixture.py 95.70% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@asifzubair
Copy link
Contributor Author

asifzubair commented Jul 28, 2025

Hi @ricardoV94 , Thank you for your comments. I've addressed them now

However, I did a rebase and push (as recommended) but it seems the rebase has added new commits on my PR ? Would you have a suggestion on how to handle this ? I can start a new PR in the worst case. Thank you 🙏

EDIT: I had to troubleshoot a bit, but now the PR looks to be in good state. Sorry for the alarm. :)

@asifzubair asifzubair force-pushed the asifzubair/fix-for-issue-7762 branch from 723ba71 to 65fb015 Compare July 28, 2025 18:15
Copy link
Member

@ricardoV94 ricardoV94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good just a small cleanup suggestion on the test

@asifzubair asifzubair force-pushed the asifzubair/fix-for-issue-7762 branch from 65fb015 to 2102cb9 Compare July 28, 2025 23:50
@ricardoV94 ricardoV94 changed the title Fix for #7762: AdvancedSubTensor with None and integer raises a logp error instead of silently failing Fix bug in mixture logprob inference Jul 29, 2025
@ricardoV94 ricardoV94 merged commit 7929b71 into pymc-devs:main Jul 29, 2025
26 checks passed
@ricardoV94
Copy link
Member

Thanks @asifzubair

@ricardoV94 ricardoV94 changed the title Fix bug in mixture logprob inference Fix bug in mixture logprob inference with None indices Jul 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug hackathon Suitable for hackathon logprob

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: AdvancedSubTensor with None and integer indices raises a logprob error instead of silently failing

2 participants