Skip to content

Consolidate Enzyme tests#469

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

Consolidate Enzyme tests#469
kshyatt wants to merge 4 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 and others added 2 commits July 8, 2026 11:06
Co-authored-by: Lukas Devos <ldevos98@gmail.com>
@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 requires formatting changes to meet the project's style guidelines.
Please consider running Runic (git runic main) to apply these changes.

Click here to view the suggested changes.
diff --git a/test/enzyme/indexmanipulations.jl b/test/enzyme/indexmanipulations.jl
index 3d47cfc..8e47e4c 100644
--- a/test/enzyme/indexmanipulations.jl
+++ b/test/enzyme/indexmanipulations.jl
@@ -67,7 +67,7 @@ Tβs = is_ci ? (Active,) : (Active, Const)
         end
     end
     if !Sys.iswindows() && VERSION > v"1.11.0-rc"
-        has_braiding && @timedtestset "flip_n_twist! TA ($TA)" for TA in (Duplicated,) 
+        has_braiding && @timedtestset "flip_n_twist! TA ($TA)" for TA in (Duplicated,)
             A = randn(T, V[1] ⊗ V[2] ← (V[3] ⊗ V[4] ⊗ V[5])')
             if !(T <: Real && !(sectorscalartype(sectortype(A)) <: Real))
                 EnzymeTestUtils.test_reverse(twist!, TA, (copy(A), TA), (1, Const); atol, rtol, fkwargs = (inv = false,))
@@ -81,7 +81,7 @@ Tβs = is_ci ? (Active,) : (Active, Const)
             EnzymeTestUtils.test_reverse(flip, TA, (A, TA), ([1, 3], Const); atol, rtol)
         end
     end
-    
+
     @timedtestset "insert and remove units TA ($TA)" for TA in (Duplicated,)
         A = randn(T, V[1] ⊗ V[2] ← (V[3] ⊗ V[4] ⊗ V[5])')
         @testset for insertunit in (insertleftunit, insertrightunit)

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