Skip to content

Consolidate Enzyme tests#469

Open
kshyatt wants to merge 6 commits into
mainfrom
ksh/enz_consolidate
Open

Consolidate Enzyme tests#469
kshyatt wants to merge 6 commits into
mainfrom
ksh/enz_consolidate

Conversation

@kshyatt

@kshyatt kshyatt commented Jun 30, 2026

Copy link
Copy Markdown
Member

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.

@kshyatt
kshyatt force-pushed the ksh/enz_consolidate branch 2 times, most recently from 0f9729f to 4396568 Compare July 1, 2026 14:13
Comment thread test/enzyme/linalg.jl Outdated
Comment on lines +29 to +35
rTαs = if α === Zero()
(Const,)
elseif !is_ci
(Active, Const)
else
(Active,)
end

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I kind of meant to change it everywhere but got pulled into a meeting midway through my suggestions 😁

@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
see 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kshyatt
kshyatt force-pushed the ksh/enz_consolidate branch from bef6af8 to 513299c Compare July 8, 2026 15:06
@kshyatt kshyatt changed the title Consolidate linalg and VI tests Consolidate Enzyme tests Jul 8, 2026
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Your PR no longer requires formatting changes. Thank you for your contribution!

@kshyatt
kshyatt force-pushed the ksh/enz_consolidate branch from c49d381 to 1cda8b4 Compare July 13, 2026 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants