-
Notifications
You must be signed in to change notification settings - Fork 140
Use vectorized jacobian in Minimize Op #1582
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
Use vectorized jacobian in Minimize Op #1582
Conversation
pre-commit.ci autofix |
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (88.88%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1582 +/- ##
=======================================
Coverage 81.63% 81.64%
=======================================
Files 231 231
Lines 52993 52998 +5
Branches 9395 9395
=======================================
+ Hits 43263 43268 +5
- Misses 7281 7282 +1
+ Partials 2449 2448 -1
🚀 New features to boost your workflow:
|
pre-commit.ci autofix |
Can you open an issue for the fact the optimization notebook now fails with these changes? |
Will do, but I can't seem to find the specific block that fails, most of these seem to run fine for me? Didn't test all of them though (I assume this is the notebook you're referring to?): https://pytensor.readthedocs.io/en/latest/gallery/optimize/root.html |
The nb runs top to bottom correctly after the PR? Can you confirm one of the dprint graphs changed now that the Jacobian doesn't use scan? Otherwise you may not be running with the changes from here |
Ah, one of the blocks all the way near the bottom fails:
I'll raise an issue now. |
@ricardoV94 is the notebook failure a blocker for this PR? |
No but we need to remember to rerun when we fix the bug |
ee37c24
to
5859a2e
Compare
I don't think the bug is related to this PR. But I changed this to an option in minimize and root (since I don't think vectorize will always be what we want), so this is good to go after tests pass imo. |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
dfe8768
to
da10fbc
Compare
@ricardoV94 after I rebased I got some cache key errors locally in the optimize tests and had to do |
There were major changes but cache errors sounds like a bug. Do you know which Op or what function we can try to compile to reproduce? |
No, I intelligently didn't save the error message, just cleared the cache then re-ran it. I guess we'll see if anyone else hits it. |
Do you know if it was when running some test here or a specific notebook? |
Test here |
Refactors as part of pymc-devs/pymc-extras#532
📚 Documentation preview 📚: https://pytensor--1582.org.readthedocs.build/en/1582/