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

Fix bug in compute_log_likelihood when variable has dims without coords #6882

Merged
merged 1 commit into from
Sep 14, 2023

Conversation

jaharvey8
Copy link
Contributor

@jaharvey8 jaharvey8 commented Aug 29, 2023

Bugfixes

Fixes #6820


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

@codecov
Copy link

codecov bot commented Aug 29, 2023

Codecov Report

Merging #6882 (6e96bf4) into main (f77372c) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6882      +/-   ##
==========================================
- Coverage   92.16%   92.16%   -0.01%     
==========================================
  Files         100      100              
  Lines       16841    16839       -2     
==========================================
- Hits        15521    15519       -2     
  Misses       1320     1320              
Files Changed Coverage Δ
pymc/model/core.py 91.29% <ø> (ø)
pymc/backends/arviz.py 96.51% <100.00%> (+0.03%) ⬆️
pymc/sampling/jax.py 98.27% <100.00%> (-0.03%) ⬇️
pymc/stats/log_likelihood.py 100.00% <100.00%> (ø)

@ricardoV94
Copy link
Member

Thanks @jaharvey8. Can you add a small test that was triggering the error before this fix?

@jaharvey8
Copy link
Contributor Author

Sure, I hate to ask an annoying question, but what's the best format for uploading a test? Or is there documentation somewhere on uploading tests?

@ricardoV94 ricardoV94 added the bug label Aug 29, 2023
@ricardoV94
Copy link
Member

Sure, I hate to ask an annoying question, but what's the best format for uploading a test? Or is there documentation somewhere on uploading tests?

Not annoying at all. There should some tests in tests/stats/test_log_likelihood.py (or something similar). You can add a separate test function for this bug there.

To run the tests locally check this guide: https://www.pymc.io/projects/docs/en/latest/contributing/running_the_test_suite.html

If you have any questions, just ask away!

@ricardoV94 ricardoV94 changed the title Updated coord processing in stats/log_likelihood.py Fix bug in compute_log_likelihood when variable has dims without coords Aug 29, 2023
@ricardoV94
Copy link
Member

@jaharvey8 I refactored a bit the code to avoid duplication and added a test. Let me know if it looks good on your end.

@ricardoV94 ricardoV94 merged commit 9227827 into pymc-devs:main Sep 14, 2023
22 checks passed
@ricardoV94 ricardoV94 added the trace-backend Traces and ArviZ stuff label Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug trace-backend Traces and ArviZ stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: object of type 'NoneType' has no len() when computing log likelihood
3 participants