Skip to content

Fix memory leak in logp of transformed variables #6991

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

Merged
merged 3 commits into from
Nov 8, 2023

Conversation

ricardoV94
Copy link
Member

@ricardoV94 ricardoV94 commented Nov 6, 2023

Closes #6990

First commit is just refactoring, second commit fixes the linked issue


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

Copy link

codecov bot commented Nov 6, 2023

Codecov Report

Merging #6991 (b487baa) into main (ec4407d) will increase coverage by 4.39%.
The diff coverage is 94.01%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6991      +/-   ##
==========================================
+ Coverage   87.78%   92.18%   +4.39%     
==========================================
  Files         100      101       +1     
  Lines       16896    16907      +11     
==========================================
+ Hits        14832    15585     +753     
+ Misses       2064     1322     -742     
Files Coverage Δ
pymc/logprob/basic.py 94.32% <100.00%> (+0.04%) ⬆️
pymc/logprob/transforms.py 95.30% <100.00%> (+0.32%) ⬆️
pymc/logprob/transform_value.py 93.75% <93.75%> (ø)

... and 10 files with indirect coverage changes

@ricardoV94 ricardoV94 force-pushed the fix_transform_values_new_op branch from 431dedb to 519009b Compare November 6, 2023 12:57
@ricardoV94 ricardoV94 force-pushed the fix_transform_values_new_op branch from 519009b to 48867de Compare November 6, 2023 14:45
@ricardoV94
Copy link
Member Author

ricardoV94 commented Nov 6, 2023

Weird failure on Windows, seems related to the progress_bar. Something akin to: AnswerDotAI/fastprogress#37

https://github.com/pymc-devs/pymc/actions/runs/6772363062/job/18404776473?pr=6991#step:7:197

@ricardoV94 ricardoV94 requested a review from twiecki November 7, 2023 12:06
@ricardoV94 ricardoV94 marked this pull request as ready for review November 7, 2023 12:06
@@ -28,6 +29,8 @@
from pymc.smc.kernels import IMH, systematic_resampling
from tests.helpers import assert_random_state_equal

_IS_WINDOWS = platform.system() == "Windows"
Copy link
Member Author

Choose a reason for hiding this comment

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

@aloctavodia I had to disable the progressbar on windows. Tests were failing with a cryptic error that I feel is unrelated?

#6991 (comment)

I couldn't get on a Windows machine to reproduce.

@ricardoV94 ricardoV94 force-pushed the fix_transform_values_new_op branch from 9a1c936 to b487baa Compare November 8, 2023 09:41
@ricardoV94 ricardoV94 merged commit 6b051f9 into pymc-devs:main Nov 8, 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.

BUG: logp of transformed variables leaks memory
2 participants