Move AD backends to standalone integration test suites#470
Merged
Conversation
The Enzyme AD backend has its own integration test environment with its own Project.toml (mirroring Mooncake.jl's pattern). All Enzyme test coverage — `find_alpha` rule, VecCorrBijector, VecCholeskyBijector, PlanarLayer, PDVecBijector — now lives there. The main test suite and `default_adtypes` are Enzyme-free, removing the runtime `Pkg.add` hack in runtests.jl and the per-file Enzyme skip shims. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each AD test body lives in one place under `test/shared/` (helpers, bijector test functions, and vector-distribution lists with parameterised `test_all` wrappers). The main suite and the Enzyme integration test each pick their adtype lists and call the same shared helpers, eliminating ~550 lines of near-duplicate test code across `test/ad/*.jl`, `test/vector/*.jl`, and `test/integration/enzyme/main.jl`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the three-file `test/shared/` directory with a single
`test/testresources.jl`. Tests are now exposed as `ADTestCase` and
`VectorTestCase` values keyed by tag and retrieved via
`generate_testcases(Val{:tag})`, mirroring Mooncake's TestCase pattern.
Each tag — `:veccorrbijector`, `:veccholeskybijector`, `:planarlayer`,
`:pdvecbijector`, `:stackedbijector` for the bijector AD tests; nineteen
distribution tags for the VectorBijectors `test_all` cases — returns a
list of typed test cases. Callers loop over the cases and run them
through `run_ad_case` / `run_vector_case` with whichever adtype list
applies.
The vector test files no longer need a module wrapper, since the
distribution lists they used to own now live in `testresources.jl`;
each `test/vector/*.jl` is a single scope-local `@testset` block.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The last two `:type_unstable_products` cases — `product_distribution(p1t, p1t, p1t)` and `product_distribution(p1a, p1a, p1a)` — were `enzyme_failures` on `main`: Enzyme can't differentiate through these triple-nested tuple-of-products. Run them in the main suite (non-Enzyme backends handle them fine) and slice them out in `test/integration/enzyme/main.jl` so the integration job doesn't flap on known failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Filtering `:type_unstable_products` cases by trailing index would break silently if anyone reordered the list in `test/testresources.jl`. Detect the failing pattern structurally instead — `ProductDistribution` whose `.dists` is a `Tuple` of `Product` / `ProductDistribution` components, which is the triple-nested shape Enzyme rejects. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Use `Bijectors.VectorBijectors._name` (via a `_case_name` helper) for
`VectorTestCase.name`. Replaces the 1000-character `string(d)` output
for nested products with a compact form like `LKJCholesky` or
`OrderStatistic(Normal, 10, 1)`.
- Add a `VectorTestCase(dist; kwargs...)` constructor that defaults the
name to `_case_name(dist)`, dropping the explicit name arg from every
list comprehension.
- Centralise the four-element non-Enzyme adtype list as
`NONENZYME_ADTYPES` in `testresources.jl`. The eight `test/vector/*.jl`
files used to declare their own identical copy.
- Hoist `cases = generate_testcases(...)` above backend loops in
`test/ad/{corr,flows,pd,stacked}.jl` and `test/integration/enzyme/main.jl`
so each tag's random inputs are constructed once per testset, not once
per backend.
- Inline the single-use `reshaped_beta_pre_111_backends` constant and
move `_enzyme_failing_product` to top-level scope in the integration
test.
- Drop the unused `using ForwardDiff: ForwardDiff` import and the
redundant `DifferentiationInterface.gradient` qualification.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Name the const for what it is (the baseline shared adtype list) rather than what it excludes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- `.github/workflows/Enzyme.yml`: drop `Pkg.resolve()` from the run step. It was breaking the fresh runners with "expected package ForwardDiff to be registered" — `Pkg.resolve` runs before the registry is fetched. `Pkg.instantiate()` alone (which auto-fetches the registry and builds the Manifest from Project.toml) is the right call. - Run JuliaFormatter on `test/testresources.jl` and `test/integration/enzyme/main.jl` so the Format CI check passes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Restore `using ForwardDiff: ForwardDiff` in the Enzyme integration script. Without it loaded, DI can't dispatch `AutoForwardDiff()` (the reference backend that `VectorBijectors.test_all` uses for optics / logjac / AD-correctness checks), so every distribution test errors on Julia 1.11 with "import ForwardDiff … and try again". - `continue-on-error: true` on the Enzyme job. Enzyme is broken on `main` by design of this PR — the integration suite surfaces failures (e.g. the 1.10 Reverse-mode VecCholesky segfault) without making the workflow red and blocking PRs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- `ADTestCase.broken=true` now runs the real gradient comparison under `@test_broken`. If the upstream bug heals, the case flips to "unexpectedly passing" instead of staying silently broken forever. - Add `broken::Bool` field to `VectorTestCase`; `run_vector_case` emits a `@test_broken false` for these (kept as a hard skip since `test_all` doesn't have a single pass/fail). - Construct randomised AD test inputs through `StableRNG(23)` so every backend exercising a given tag sees identical inputs and CI reruns reproduce. Threads through `:veccorrbijector`, `:veccholeskybijector`, `:planarlayer`, `:pdvecbijector`, `:stackedbijector`. - Drop the tag enumeration from the `test_resources.jl` header — the method table of `generate_testcases` is the source of truth. - Rename `test/testresources.jl` -> `test/test_resources.jl`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1. Drop redundant `using DifferentiationInterface: gradient` — already
re-exported by `using DifferentiationInterface`.
2. Use the new `broken` field for the Enzyme `enzyme_failures` skip in
test/integration/enzyme/main.jl: the offending products now appear as
broken in the test report rather than being silently filtered.
3. Convert every single-statement `@testset "..." for c in cases …` to
block form in the integration test and `test/vector/*.jl` so the
testset shape is uniform across the suite.
4. Rename order-statistic test cases to sentence case matching the
`Bijectors.VectorBijectors._name` style ("order statistic Normal
i=1 of n=10" / "joint order statistic Normal n=4 (all ranks)" /
"ordered MvNormal"), so test output reads consistently with the
auto-named cases for other distributions.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the pointer-style comment that mapped each backend list to its historical origin on `main` with one that explains what each flag (Const, runtime activity) actually does and which class of tests each flavour covers. Reads on its own without needing to chase the old code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the per-flag/per-flavour explanation with a brief note that `set_runtime_activity` is the load-bearing flag and the rest are configurations each test was validated against. Avoids the prior comment's overreaching technical claims (that without `Const` Enzyme always threads tangents through captures, that `set_runtime_activity` replaces compile-time inference, etc.) which empirical evidence on this PR contradicts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…s deps
ReverseDiff and Mooncake now have dedicated integration suites under
test/integration/{reversediff,mooncake}, mirroring the existing Enzyme setup.
Each backend gets its own CI workflow with blocking failures (Enzyme stays
non-blocking). test/ad/ disappears: the four backend-exercise wrappers fold
into the integration mains, and the rule-correctness test moves to
test/bijectors/chainrules.jl.
DifferentiationInterface drops out of Bijectors's [deps]/[compat]. The
DI-dependent test machinery (linked_optic_vec AD check, AD-vs-analytical
logjac, test_ad) moves from src/vector/test_utils.jl into the test resources;
VectorBijectors.test_all keeps only the structural checks.
Test cases now carry a tag::Symbol; new aggregate entry points
generate_ad_testcases() and generate_vector_testcases() return flat lists.
Each integration main.jl becomes one adtype list + one is_broken(c) predicate
+ two short loops. Enzyme collapses to a single runtime_activity+Const adtype
list across all cases.
The vector test_all coverage that lived in test/vector/*.jl is now driven
through test/vector/main.jl (machinery + GROUP-guarded loop) with per-tag
generators kept in the per-category sibling files.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`test/vector/main.jl` now contains only the GROUP-aware @testset that runs `run_vector_case` over `generate_vector_testcases()`. `test/runtests.jl` includes it conditionally on GROUP. The VectorTestCase machinery (struct, runner, AD-dependent helpers, _VECTOR_TAGS, generate_vector_testcases) lives directly in `test_resources.jl`, which also includes each per-category generator file (univariate.jl, multivariate.jl, …, product.jl) individually so integration tests pick them up via a single `include("test_resources.jl")`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two follow-ups from the audit:
- test/integration/reversediff/Project.toml had an absolute [sources] path
("/Users/hg344/...") left over from a local Pkg.develop call. Restore the
relative ../../.. form so the new ReverseDiff CI workflow can instantiate
Bijectors on any checkout.
- The main suite no longer ran the bijector-level AD cases against ForwardDiff
once test/ad/* was removed. ForwardDiff is already a hard test dep — it is
the reference for both `run_ad_case` (via AutoFiniteDifferences) and for the
vector test_all_ad helpers — so keep its coverage as an inline @testset in
the Classic group: `gradient(f, AutoForwardDiff(), x)` is compared against
the finite-differences reference for every case in `generate_ad_testcases()`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…erface.jl
Several small cleanups bundled together. Verified locally: Classic group
passes 6614/6614 (1m39s); ReverseDiff integration passes 553,978/553,990
with 12 expected-broken cases (the LKJ + JointOrderStatistics tags).
- Drop DistributionsAD from test deps and remove test/distributionsad.jl;
strip its vestigial `using` imports from test/bijectors/{corr,pd}.jl and
test/interface.jl, plus the TuringWishart/filldist test blocks that
depended on it. The package's [weakdeps] entry is untouched.
- Rename test/integration → test/integration_tests, test/norm_flows.jl →
test/normalising_flows.jl, test/transform.jl → test/legacy_interface.jl.
Update CI workflows and runtests.jl references.
- Inline test/vector/main.jl back into runtests.jl so the main-suite
vector loop lives next to the other test entry points.
- Restore the relative [sources] path in test/integration_tests/reversediff
and add ChainRules to its deps so BijectorsReverseDiffChainRulesExt
activates and Bijectors.cholesky_lower/cholesky_upper resolve under
ReverseDiff.
- Split test/interface.jl into per-feature bijector files under
test/bijectors/: Stacked (appended to existing stacked.jl, which was
previously not even included from runtests.jl), Jacobians of
SimplexBijector → simplex.jl, Equality → equality.jl, the
test_inverse / test_with_logabsdet_jacobian smoke → scale.jl. Drop the
4-line "Example: ADVI single" smoke (covered by the Stacked ADVI block).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- test/bijectors/simplex.jl: drop the two trailing `Δ_forwarddiff` /
`Δ_forwarddiff_inv` gradient computations. They were left over from a
pre-Tracker comparison that was deleted earlier; the values were never
asserted on, so the lines (and their misleading "ensure pullbacks are
the same" comment) are dead code.
- test/test_resources.jl: update header reference to point at
test/runtests.jl (the file that hosts the main-suite inline vector
loop) instead of the now-deleted test/vector_bijectors.jl.
- test/interface.jl: refresh the stale "similar to what we do in
test/transform.jl" comment to track the rename.
- test/integration_tests/enzyme/main.jl: unify `is_broken` into a single
`Union{VectorTestCase,ADTestCase}` method, matching the shape used by
the Mooncake and ReverseDiff suites.
- test/bijectors/equality.jl: prefer `eachindex(bs)` over `1:length(bs)`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Bijectors.jl documentation for PR #470 is available at: |
Replace explicit 1.10/1.11 entries with "min" and "1" so the matrix tracks the compat lower bound and latest stable Julia automatically. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop "AD tests" suffix from per-backend workflow names and give each job a concise display name. Tiles now read e.g. "Mooncake / Julia 1" instead of "Mooncake AD tests / mooncake (1)". CI.yml gets a similar treatment so the runner-struct no longer appears as JSON. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tile now reads "Format / runic" instead of "Format / format". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Enable doctest=true in docs/make.jl and drop the dedicated Doctests group + workflow. The Documenter docs build now covers what DocTests.yml used to run separately. - docs/make.jl: doctest=true, hoist the └ @ filter into doctestfilters - test/runtests.jl: drop the Doctests testset and the Documenter import - test/Project.toml: drop Documenter (no longer needed in main tests) - delete .github/workflows/DocTests.yml Verified `julia --project=docs docs/make.jl` runs doctests and the build completes cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ermission Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
@sunxd3, a quick look should be enough -- there is no functionality change or loss of tests here (verified), it is only restructuring tests. |
The [sources] entry in each AD integration test's Project.toml is a Julia 1.11+ feature; on Julia 1.10 it's silently ignored and the registry version of Bijectors is resolved instead, which produces stale test_utils.jl errors. Pkg.develop forces the working tree across all supported Julia versions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MacroTools is not imported anywhere in the test suite; it remains available transitively through other dependencies that genuinely need it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
why are you considering Enzyme differently here? |
Member
|
Two points worth noting:
|
DistributionsAD.jl is archived (last release v0.6.58, March 2025) and Turing v0.43 deprecated it as a dependency. No current TuringLang package constructs TuringDirichlet/FillVectorOfUnivariate/etc., and external usage doesn't go through Bijectors.bijector. The extension's dispatches have been unused and untested since the test cleanup earlier in this PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
Thanks @sunxd3, both good catches. (2) DistributionsAD extension — dropped in 24ed323. Verified it's safe: DistributionsAD.jl is archived (last release March 2025); Turing v0.43 explicitly deprecated it.
EDIT: I removed the changes to |
Restore DifferentiationInterface as a hard Bijectors dependency and fold test_linked_optic, test_linked_logjac, test_ad, the to_vec_for_logjac_test/from_vec_for_logjac_test family, and _rand_safe_ad back into src/vector/test_utils.jl. test_all gains adtypes / ad_atol / ad_rtol kwargs and defaults adtypes to [AutoForwardDiff()] so the main Vector suite exercises FD as the baked-in reference. Integration suites pass their own adtypes list through the (now overloaded) run_vector_case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Revert src/vector/test_utils.jl to main; only change default_adtypes to [ref_adtype] (ForwardDiff-only). Drop the now-unused EnzymeCore import. - Merge the two run_vector_case methods in test/test_resources.jl into one with an optional adtypes positional arg. - Trim narrative/history-note comments in test_resources.jl, runtests.jl, test/vector/product.jl, test/vector/reshaped.jl. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
|
thanks Hong. couple of follow-ups:
|
`test_all` defaults to `[AutoForwardDiff()]`; ReverseDiff, Mooncake, and Enzyme live in the per-backend integration suites under test/integration_tests/. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`test_all` gains `broken::Bool=false` and `skip::Bool=false`. `skip` short- circuits the AD section; `broken` routes to a new `_test_ad_broken` that collapses the four per-adtype AD comparisons (forward/reverse × jacobian / log-abs-det-Jacobian) into a single `@test_broken`, so a partially-broken upstream backend stays cleanly "broken" instead of spamming "unexpectedly passing" on whichever sub-check happens to work. `test_ad` itself stays close to main except for one simplification: discrete distributions short-circuit at the top (their link transforms are identities; nothing AD-meaningful to check) instead of running the prior backend filter. The integration suites are switched to a per-suite `vector_is_broken(c)` predicate; ReverseDiff marks `(:lkj_matrix_dists, :order_joint)`, Enzyme marks the Julia-1.10 `:reshaped_beta_special` case and triple-nested products, Mooncake marks nothing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The y → x → ynew roundtrip is flaky for JointOrderStatistics: extreme random `y` values push `x` to the support boundary, where the inverse loses enough precision that `_isapprox_safe(y, ynew)` fails even with the existing `roundtrip_atol=1e-1`. The prior special case only skipped on NaN/Inf, which doesn't catch boundary-pinned but finite roundtrips (issue #441). Structural correctness is already covered by `test_roundtrip` (x → to_vec → from_vec → x). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nularity
`test_all` now takes `broken_adtypes::Vector{<:DI.AbstractADType}` instead of
a single `broken::Bool`. Adtypes appearing in `broken_adtypes` go through the
conjunction-style `_test_ad_broken`; the rest of `adtypes` run normally under
`test_ad`. `skip::Bool` is unchanged.
This fixes the Enzyme `:reshaped_beta_special` case on Julia 1.10: only the
Reverse mode hits Enzyme.jl#2987, so it goes into `broken_adtypes` while
Forward mode still runs normally. Previously the coarse `broken=true` flag
wrapped both modes and Forward's actual pass became a noisy "Unexpected Pass"
that turned the Enzyme integration job red.
Integration suites switch from `vector_is_broken(c)::Bool` to
`vector_broken_adtypes(c)::Vector`; ReverseDiff returns the full `adtypes`
list for its tagged-broken cases, Enzyme returns `[ENZYME_REVERSE]` for the
1.10-only case.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
@sunxd3 all comments addressed - this should be ready for a final look. See the commit message for what has changed. |
sunxd3
reviewed
May 18, 2026
sunxd3
left a comment
sunxd3
left a comment
Member
There was a problem hiding this comment.
Couple of items worth attention.
An inconsistency between PR description and code diff is DI dependency (DI is still in package Project.toml, line 10)
Comment on lines
+661
to
+682
| @test_broken ( | ||
| isapprox( | ||
| DI.jacobian(ffwd, adtype, xvec), ref_jac_fwd; atol=atol, rtol=rtol | ||
| ) && | ||
| isapprox( | ||
| DI.gradient(ladj_fwd, adtype, xvec), | ||
| ref_grad_ladj_fwd; | ||
| atol=atol, | ||
| rtol=rtol, | ||
| ) && | ||
| isapprox( | ||
| DI.jacobian(frvs, adtype, yvec), ref_jac_rev; atol=atol, rtol=rtol | ||
| ) && | ||
| isapprox( | ||
| DI.gradient(ladj_rev, adtype, yvec), | ||
| ref_grad_ladj_rev; | ||
| atol=atol, | ||
| rtol=rtol, | ||
| ) | ||
| ) | ||
| end | ||
| end |
Member
There was a problem hiding this comment.
maybe split these to give more visibility to failure
Member
Author
There was a problem hiding this comment.
It's not easy to do that at the moment due to VectorBijectors's current testing setup -- we prefer to minimise changes to the src folder in this PR.
| ChainRulesCore = "0.10.11, 1" | ||
| ChangesOfVariables = "0.1" | ||
| DifferentiationInterface = "0.7.14" | ||
| DifferentiationInterface = "0.7.7" |
Member
There was a problem hiding this comment.
is there a reason for lowering this bound?
Member
Author
Member
Author
|
@sunxd3 all addressed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No changes to
src, only tests restructuring. Supersede #469workflow. ReverseDiff and Mooncake block PRs; Enzyme reports failures for visibility but is continue-on-error. The main suite covers ForwardDiff inline (it's the AD reference for both
run_ad_case and the vector AD loops).
src/vector/test_utils.jl into shared test resources; VectorBijectors.test_all keeps only the structural (DI-free) checks.
generate_vector_testcases(). Each integration main.jl reduces to one adtype list + one is_broken(c) predicate + two short loops. Enzyme collapses to a single runtime_activity + Const adtype
across all cases; product-bijector Enzyme failures are skipped by a structural predicate, not by index.
test/bijectors/ (stacked.jl, simplex.jl, equality.jl, scale.jl). Renames: test/integration → test/integration_tests, test/norm_flows.jl → test/normalising_flows.jl, test/transform.jl →
test/legacy_interface.jl. JuliaFormatter v1.0 applied across the board.