Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove find_contrained_prior #7680

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aloctavodia
Copy link
Member

@aloctavodia aloctavodia commented Feb 13, 2025

find_contrained_prior was deprecated in v5.17.0 (#7458), released on Oct 3, 2024. This PR removes it completely.


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

Copy link

codecov bot commented Feb 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.66%. Comparing base (358b825) to head (c66901d).
Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7680      +/-   ##
==========================================
- Coverage   92.70%   92.66%   -0.05%     
==========================================
  Files         107      106       -1     
  Lines       18391    18279     -112     
==========================================
- Hits        17050    16938     -112     
  Misses       1341     1341              
Files with missing lines Coverage Δ
pymc/__init__.py 100.00% <ø> (ø)

... and 2 files with indirect coverage changes

@twiecki
Copy link
Member

twiecki commented Mar 10, 2025

Can you link the issue about the deprecation?

@ricardoV94
Copy link
Member

How long ago was v5.17 released? Time is more important than version numbers

@aloctavodia
Copy link
Member Author

Comment updated

@twiecki
Copy link
Member

twiecki commented Mar 10, 2025

I now I should have commented on the original issue but can't we add the max entropy condition to find_constrained_prior? I'm not even sure what the alternative workflow is to do this with preliz but it seems to require it's own, different set of distributions.

@aloctavodia
Copy link
Member Author

PreliZ's distributions have a to_pymc method

@twiecki
Copy link
Member

twiecki commented Mar 10, 2025

How difficult would it be to port max_ent to find_constrained_prior?
Can you share example code of how this would look like in preliz?

@aloctavodia
Copy link
Member Author

aloctavodia commented Mar 10, 2025

If you want to rewrite find_constrained_prior to behave like maxent, then entropy needs to be added to distributions and then I would recommend to also use an initialization routine like the one used in maxent so users don't have to guess initial values themselves.

This will compute the maxentropy Gamma distribution that has 90% of the mass between 1 and 10, and it will return it as a PyMC distribution, that you can then use as usual.

pz.maxent(pz.Gamma(), 1, 10, 0.9).to_pymc()

You can also do other stuff like fix some of the parameters (mu in this example)

pz.maxent(pz.Gamma(mu=4), 1, 10, 0.9)

Or stats like median or mode,

pz.maxent(pz.Beta(), 0.1, 0.7, 0.94, fixed_stat=("mode", 0.3))

More examples here. https://preliz.readthedocs.io/en/latest/examples/gallery/direct_elicitation_1D.html

PreliZ's distributions follow the same parameterization used by PyMC. For instance, Gamma can be defined using, alpha-beta, or mu-sigma.

@twiecki
Copy link
Member

twiecki commented Mar 10, 2025

Would it be too hacky to replace find_constrained_prior with a call to preliz? I assume since you have code that translates preliz distribution to pymc you could have a function to tranlsate a pymc distribution into preliz.

@aloctavodia
Copy link
Member Author

We did that with arviz for a long time, so as hacky as that.
There is no from_pymc method, but I guess it should not be that hard.

@twiecki
Copy link
Member

twiecki commented Mar 10, 2025

I think that could be worth considering, just because find_constrained_prior is quite useful and also by now used in quite some training materials. And just dropping it without giving users an obvious replacement doesn't seem like a great experience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants