-
Notifications
You must be signed in to change notification settings - Fork 137
Reduce number of slow tests and balance parts #1569
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1569 +/- ##
==========================================
- Coverage 81.54% 81.53% -0.01%
==========================================
Files 230 230
Lines 53088 53088
Branches 9427 9427
==========================================
- Hits 43288 43286 -2
- Misses 7365 7366 +1
- Partials 2435 2436 +1 🚀 New features to boost your workflow:
|
1976ee4
to
e929c31
Compare
e929c31
to
2a97306
Compare
@@ -11,6 +11,9 @@ | |||
RTOL = ATOL = 1e-6 if floatX.endswith("64") else 1e-3 | |||
|
|||
|
|||
@pytest.mark.skip( |
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.
Let's just delete it inside of marking it as skip?
Maybe keep one simple smoke test (with basic settings) to make sure something upstream hasn't somehow broken something
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 want to come back to it when we implement numba caching, it's a good case of painfully slow (while it's like 2s on jax)
CI part split
SecondBroadcast tests
In general these broadcast tests are really wasteful, since they keep testing functionality that is shared for any Elemwise, regardless of the core_op, when testing the scalar op should suffice. I removed the 90 combinations we were running just to see if runtime broadcast would raise (it does ofc).
Also the chain from iterable for good tests was not doing anything, because we were not using the prefix in the helper function, so the dictionary keys were the same and overriding the first two iterables. My changes there don't actually reduce the number of iterations.
Numba pad tests
Numba CI are alwas the slowest and pad is at least 15% of it.
Since this is only testing OpFromGraph (there's no custom dispatch for pad), it seems unnecessary. There's nothing new added to the numba module because of pad
Related to #1433
Related to #1124
📚 Documentation preview 📚: https://pytensor--1569.org.readthedocs.build/en/1569/