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

Refactor get_tau_sigma and support lists of variables #7185

Merged
merged 2 commits into from
Mar 4, 2024

Conversation

tvwenger
Copy link
Contributor

@tvwenger tvwenger commented Mar 4, 2024

What is this PR about?
This PR refactors the get_tau_sigma logic in order to support lists of Variables and fixes #6987.

N.B. The original code had a positivity check for non-Variable arguments, but not for Variable arguments. Since this PR casts the arguments to Variable, I've removed the positivity check for non-Variable arguments.

Resubmission of #6988

Checklist

Major / Breaking Changes

  • get_tau_sigma now always returns a Variable, even when scalars are passed

New features

  • N/A

Bugfixes

Documentation

  • N/A

Maintenance

  • Updated tests

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

@tvwenger
Copy link
Contributor Author

tvwenger commented Mar 4, 2024

@ricardoV94 other branch got mucked up during upstream rebase, so I closed that PR in favor of this one. It's ready for review when the checks are complete.

Copy link

codecov bot commented Mar 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.26%. Comparing base (47f6d9e) to head (dfec0af).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7185      +/-   ##
==========================================
+ Coverage   92.24%   92.26%   +0.02%     
==========================================
  Files         100      100              
  Lines       16888    16880       -8     
==========================================
- Hits        15578    15574       -4     
+ Misses       1310     1306       -4     
Files Coverage Δ
pymc/distributions/continuous.py 97.79% <100.00%> (+0.25%) ⬆️

... and 1 file with indirect coverage changes

@tvwenger
Copy link
Contributor Author

tvwenger commented Mar 4, 2024

I had to revert the behavior for negative tau since it broke other tests.

@tvwenger
Copy link
Contributor Author

tvwenger commented Mar 4, 2024

@ricardoV94 Ready to merge

@ricardoV94 ricardoV94 merged commit 244fb97 into pymc-devs:main Mar 4, 2024
23 checks passed
@ricardoV94 ricardoV94 added the bug label Mar 4, 2024
@ricardoV94
Copy link
Member

Thanks @tvwenger

mkusnetsov pushed a commit to mkusnetsov/pymc that referenced this pull request Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: get_tau_sigma does not support list of variables
2 participants