-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
test on windows and mac #4269
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
test on windows and mac #4269
Conversation
411331c
to
de938fc
Compare
Codecov Report
@@ Coverage Diff @@
## master #4269 +/- ##
==========================================
- Coverage 87.69% 87.64% -0.05%
==========================================
Files 88 88
Lines 14355 14362 +7
==========================================
- Hits 12588 12587 -1
- Misses 1767 1775 +8
|
de938fc
to
e2f7764
Compare
The stack overflow 🐛 of
|
99185d7
to
907884d
Compare
@MarcoGorelli looks like we both made the same mistake in resolving the merge conflict? I think I have it right now.. Can push it if you want |
It's OK, done 😄 Let's see what errors mac / windows throw now... |
seems like mac tests pass! |
tl:dr: The hashable helper function did not appropiately deal with tuples (and the test case did not actually test the memoization). In the process of prior-predictive sampling a model involving a DensityDist, the _compile_theano_function function was called with arguments (sd__log_, []). The _compile_theano_function has a pm.memo.memoize-decorator, which relies on the pm.memo.hashable for hashing of typically unhashable objects. The "hashable" function incorrectly handled tuples, eventually causing a stackoverflow error on Windows.
Fix the stackoverflow problem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed two small glitches, but with them fixed I think we're good to go!
This is amazing! |
Let's see what happens
Stackoverflow error on Windows 🤔