Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Implement specialized Hurdle distribution#7810

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ricardoV94 merged 1 commit intopymc-devs:mainfromricardoV94:hurdle_mixtures
Jun 16, 2025

Conversation

@ricardoV94
Copy link
Member

@ricardoV94ricardoV94 commentedJun 4, 2025•
edited
Loading

It indirectly addresses the issue reported in inpymc-devs/nutpie#163

The new objects have a logp that handles the discrete + continuous process correctly, without requiring the arbitrary truncation of the latter at epsilon. This provides a cheaper and more stable logp / logcdf.
For discrete variables we keep using a truncation

Also added special logic to truncate a Hurdle distribution which solvesbambinos/bambi#768, this is not the desired behavior, reverted it

CC@zwelitunyiswa

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

@ricardoV94ricardoV94force-pushed thehurdle_mixtures branch 2 times, most recently from77ed668 to9c65ca1CompareJune 4, 2025 14:02
@ricardoV94ricardoV94 changed the titleImplement specialized Hurdle distributionImplement specialized Hurdle distribution and allow truncating itJun 4, 2025
@ricardoV94ricardoV94force-pushed thehurdle_mixtures branch 3 times, most recently from3d5772c tobdb3f12CompareJune 4, 2025 14:15
@ricardoV94ricardoV94force-pushed thehurdle_mixtures branch 2 times, most recently fromac73b55 to9a65487CompareJune 4, 2025 14:52
@pymc-devspymc-devs deleted a comment fromreview-notebook-appbotJun 4, 2025
dist
fordistindists
if (
getattr(dist,"rv_type",None)isnotNone
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

This was too restrictive, a subclass also inherits the dispatch function, and need not be in the registry explicitly

lucianopaz reacted with thumbs up emoji
@codecov
Copy link

codecovbot commentedJun 4, 2025•
edited
Loading

Codecov Report

❌ Patch coverage is91.42857% with6 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.88%. Comparing base (3b62f82) to head (1e38719).
⚠️ Report is 48 commits behind head on main.

Files with missing linesPatch %Lines
pymc/distributions/mixture.py91.17%6 Missing⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@##             main    #7810   +/-   ##=======================================  Coverage   92.88%   92.88%           =======================================  Files         107      107             Lines       18377    18389   +12     =======================================+ Hits        17069    17081   +12  Misses       1308     1308
Files with missing linesCoverage Δ
pymc/distributions/moments/means.py100.00% <100.00%> (ø)
pymc/distributions/mixture.py95.25% <91.17%> (+0.23%)⬆️
🚀 New features to boost your workflow:
  • ❄️Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ricardoV94ricardoV94 marked this pull request as draftJune 4, 2025 15:59
@zwelitunyiswa
Copy link

It indirectly addresses the issue reported in inpymc-devs/nutpie#163

The new objects have a logp that handles the discrete + continuous process correctly, without requiring the arbitrary truncation of the latter at epsilon. This provides a cheaper and more stable logp / logcdf. For discrete variables we keep using a truncation

Also added special logic to truncate a Hurdle distribution which solvesbambinos/bambi#768

CC@zwelitunyiswa

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

@ricardoV94 This is amazing. Thank you so much for this!

@ricardoV94ricardoV94 changed the titleImplement specialized Hurdle distribution and allow truncating itImplement specialized Hurdle distributionJun 5, 2025
@ricardoV94ricardoV94 marked this pull request as ready for reviewJune 5, 2025 12:34
Copy link
Contributor

@zaxtaxzaxtax left a comment

Choose a reason for hiding this comment

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

LGTM! We should probably add a test similar to one that motivate this PR in the first class

@ricardoV94
Copy link
MemberAuthor

ricardoV94 commentedJun 5, 2025•
edited
Loading

LGTM! We should probably add a test similar to one that motivate this PR in the first class

We have the pre-existing hurdlesl tests, in a sense this is just a refactor/optimization. Can't think of anythingreasonable obvious to test here?

@zaxtax
Copy link
Contributor

LGTM! We should probably add a test similar to one that motivate this PR in the first class

We have the pre-existing hurdlesl tests, in a sense this is just a refactor/optimization. Can't think of anythingreasonable obvious to test here?

What caused the error originally reported here?pymc-devs/nutpie#163 Does that have a test already?

@ricardoV94
Copy link
MemberAuthor

LGTM! We should probably add a test similar to one that motivate this PR in the first class

We have the pre-existing hurdlesl tests, in a sense this is just a refactor/optimization. Can't think of anythingreasonable obvious to test here?

What caused the error originally reported here?pymc-devs/nutpie#163 Does that have a test already?

That was fixed sometime ago in PyTensor:pymc-devs/pytensor#1137

The performance question when in numba is addressed bypymc-devs/pytensor#1445

Neither is PyMC specific

@zaxtax
Copy link
Contributor

LGTM! We should probably add a test similar to one that motivate this PR in the first class

We have the pre-existing hurdlesl tests, in a sense this is just a refactor/optimization. Can't think of anythingreasonable obvious to test here?

What caused the error originally reported here?pymc-devs/nutpie#163 Does that have a test already?

That was fixed sometime ago in PyTensor:pymc-devs/pytensor#1137

The performance question when in numba is addressed bypymc-devs/pytensor#1445

Neither is PyMC specific

Feel free to merge whenever you feel comfortable! I think it's good to go

Copy link
Member

@lucianopazlucianopaz 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 to me. I just left two very minor comments

)

returnmix_logp
mix_support_point=pt.sum(weights*support_point_components,axis=mix_axis)
Copy link
Member

Choose a reason for hiding this comment

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

Why not use the logsumexp and have log scale weights here? Is it because the weights are already in the 0-1 range and taking the log won’t help with precision?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

We're not computing any log quantities nor starting with any log quantities so I don't think it would help. Also the initial point is not so critical?

This does not require the arbitrary truncation of continuous distribution in the logp/logcdf
@ricardoV94ricardoV94 merged commit0f1bfa9 intopymc-devs:mainJun 16, 2025
40 of 42 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@zaxtaxzaxtaxzaxtax approved these changes

@lucianopazlucianopazlucianopaz approved these changes

@tomicaprettotomicaprettoAwaiting requested review from tomicapretto

+1 more reviewer

@zwelitunyiswazwelitunyiswazwelitunyiswa left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ricardoV94@zwelitunyiswa@zaxtax@lucianopaz@tomicapretto
[8]ページ先頭
©2009-2025 Movatter.jp