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 joint_logprob function from tests.logprob.utils #6650

Merged
merged 3 commits into from
May 19, 2023

Conversation

shreyas3156
Copy link
Contributor

@shreyas3156 shreyas3156 commented Apr 4, 2023

What is this PR about?
Closes #6619

The joint_logprob() function in tests.logprob.utils uses factorized_joint_logprob() and only checks for summing over the dimensions of the logp calculated. It can therefore be replaced with logp() or factorized_joint_logprob() depending on the number of RVs to be calculated, with an additional statement to sum over the dimensions in the test cases.

Checklist

Major / Breaking Changes

  • None

Maintenance

  • Removing the method should somewhat decrease the functional overhead.

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

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 spotted some test name changes and one test that should be moved or removed.

tests/logprob/test_basic.py Outdated Show resolved Hide resolved
tests/logprob/test_basic.py Outdated Show resolved Hide resolved
tests/logprob/test_basic.py Outdated Show resolved Hide resolved
@shreyas3156 shreyas3156 force-pushed the remove-joint-logprob-test-6619 branch from 6ef753b to 1b8bbfd Compare April 15, 2023 12:20
@codecov
Copy link

codecov bot commented Apr 15, 2023

Codecov Report

Merging #6650 (6ef753b) into main (1ed4475) will not change coverage.
The diff coverage is n/a.

❗ Current head 6ef753b differs from pull request most recent head 1b8bbfd. Consider uploading reports for the commit 1b8bbfd to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #6650   +/-   ##
=======================================
  Coverage   91.99%   91.99%           
=======================================
  Files          94       94           
  Lines       15944    15944           
=======================================
  Hits        14667    14667           
  Misses       1277     1277           

@ricardoV94 ricardoV94 merged commit 0073639 into pymc-devs:main May 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove joint_logprob function from tests.logprob.util submodule
3 participants