Consolidate Enzyme tests#469
Conversation
0f9729f to
4396568
Compare
| rTαs = if α === Zero() | ||
| (Const,) | ||
| elseif !is_ci | ||
| (Active, Const) | ||
| else | ||
| (Active,) | ||
| end |
There was a problem hiding this comment.
| rTαs = if α === Zero() | |
| (Const,) | |
| elseif !is_ci | |
| (Active, Const) | |
| else | |
| (Active,) | |
| end | |
| rTαs = [] | |
| α === Zero() || push!(rTαs, Active) | |
| (α === Zero() || !is_ci) && push!(rTαs, Const) |
Just a small suggestion for code style, I tend to find it a bit easier to see what is going on if there aren't too many lines
There was a problem hiding this comment.
I don't quite like that this is now changed just here in this one instance, while the test file is full of these constructs. I don't mind the three case separation and keeping rTαs a tuple (though clearly that doesn't have any real advantages here). I would suggest to revert it to the previous state, unless @lkdvos feels strongly, but then they should probably be changed everywhere
There was a problem hiding this comment.
I kind of meant to change it everywhere but got pulled into a meeting midway through my suggestions 😁
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
bef6af8 to
513299c
Compare
|
Your PR no longer requires formatting changes. Thank you for your contribution! |
Co-authored-by: Lukas Devos <ldevos98@gmail.com>
c49d381 to
1cda8b4
Compare
Fixes to the caching for CI runs and some Enzyme improvements have cut the runtime of Enzyme tests substantially, so we can probably fold these back into smaller files to reduce the amount of CI runs.