Add 68 idiomatic PyMC model implementations#319
Add 68 idiomatic PyMC model implementations#319twiecki wants to merge 3 commits intostan-dev:masterfrom
Conversation
Add PyMC translations for 68 Stan models that follow idiomatic PyMC patterns: no Potentials, no custom helper functions, and all numpy data wrangling occurs before the pm.Model context. Models span: GLMs (wells, radon, kidscore, logearn variants), GPs (gp_regr, gp_pois_regr), mixture models, educational models (eight_schools, rats, diamonds), and ecological models (seeds, dogs). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ricardoV94
left a comment
There was a problem hiding this comment.
Looking pretty nice, left some comments that apply more in general than the places I noted.
Big issues:
- Lot's of dangling commets/logic for "normalization correction/matching Stan" that is not being done anymore
- Way too many dumb comments in general, let the code speak unless it really seems pertinent
- Some models have the
prior_onlybut most don't? Is this a thing in the original posteriordb or the LLM just forgot about it? - Common to see the same expression defined once in the likelihood and then repeated inside a Deterministic. Not idiomatic at all
If we fix those, this is pretty much ready. Can you push a commit on top of this, instead of regenerating everything and force-pushing? Or even if you regenerate everything, don't force push
Address review comments on PR stan-dev#319: - Remove unnecessary comments (let the code speak) - Add prior_only parameter consistently to all 68 models - Define Deterministics before likelihood, reference in observed - Fix variable names to match semantics (e.g. logit_p not p) - Remove dangling code from removed normalization corrections - Net -464 lines (295 added for prior_only, 759 removed) Applied via: transpailer refine --skill pymc_idiomaticity --in-place Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- accel_splines: remove rename-only Deterministics, move data before model - log10earn_height: remove dangling N_obs variable - low_dim_gauss_mix_collapse: use pm.NormalMixture instead of pm.Mixture - normal_mixture: use pm.NormalMixture instead of pm.Mixture Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@ricardoV94 should be addressed. |
|
@MansMeg this should be good to go. |
|
Big thanks for the contribution! I’ll run the models through our internal test suite, which appears very similar to what you’ve been using. One initial observation: Importing the Python modules (corresponding to the PyMC models) raises a NameError on the return type hint: This happens because pm is not defined at function definition time. I suggest moving the module imports (e.g., |
| "references": "bales2019selecting", | ||
| "added_date": "2020-02-01", | ||
| "added_by": "Oliver Järnefelt", | ||
| "added_by": "Oliver J\u00e4rnefelt", |
There was a problem hiding this comment.
Please revert this change here and in subsequent files.
Summary
pm.Potential, no custom helper functionspm.Modelcontext manager.info.jsonfiles to register PyMC implementationsModel categories included
What is NOT included (for separate PR)
pm.Potential(40 models) - need review of whether Potentials can be eliminatedTest plan
pm.Potentialusage across all 68 modelsmake_model)with pm.Model()