Access full_cache, resize_non_user_cache!, AutoDePSpecialize and SciMLOperators from their owners - #4168
Conversation
…LOperators from their owners SciMLBase owns and publicly declares `full_cache` and `resize_non_user_cache!` (both `export`ed) and `AutoDePSpecialize` (`public` since 3.39.0); OrdinaryDiffEqCore only re-exports them. SciMLOperators owns the `SciMLOperators` module name, which OrdinaryDiffEqLinear reached through SciMLBase. Import and access these through their owners in the 14 sublibraries that reached them via a re-exporter, matching the OrdinaryDiffEqBDF change in SciML#4167. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
de6c961 to
fcb8ddd
Compare
|
Rebased onto current
|
CI triage — all four current failures are pre-existing on
|
| job | cause | pre-existing? |
|---|---|---|
lib/OrdinaryDiffEqLinear / Julia lts / Tests - Core |
UndefVarError: MatrixOperator at linear_method_tests.jl:6 |
yes — SciMLBase 3.40.0 dropped @reexport using SciMLOperators (SciML/SciMLBase#1472); fix in #4170 |
lib/OrdinaryDiffEqExponentialRK / Julia lts / Tests - Core |
same, at linear_nonlinear_krylov_tests.jl:21 |
yes — same, #4170 |
lib/OrdinaryDiffEqExponentialRK / Julia pre / Tests - Core |
same | yes — and julia pre is allowed-to-fail |
lib/OrdinaryDiffEqAdamsBashforthMoulton [Threaded] / Julia 1 |
precompilation failure of OrdinaryDiffEqLowOrderRK |
yes — see below |
The Threaded one is worth spelling out because it is a hard precompile error rather than a test failure, so it could easily be mistaken for a breakage introduced here. It is not: this PR touches neither OrdinaryDiffEqLowOrderRK nor OrdinaryDiffEqCore, and the error reproduces byte-identically on an unmodified master checkout at 7b1ad0368:
$ cd lib/OrdinaryDiffEqAdamsBashforthMoulton
$ GROUP=Threaded julia +1.12 --project -e 'using Pkg; Pkg.test()'
WARNING: Imported binding OrdinaryDiffEqCore.du_cache was undeclared at import time during import to OrdinaryDiffEqLowOrderRK.
WARNING: Imported binding OrdinaryDiffEqCore.u_cache was undeclared at import time during import to OrdinaryDiffEqLowOrderRK.
ERROR: LoadError: invalid method definition in OrdinaryDiffEqLowOrderRK: exported function OrdinaryDiffEqCore.u_cache does not exist
ERROR: LoadError: Failed to precompile OrdinaryDiffEqLowOrderRK [1344f307-1e59-4825-a18e-ace9aa3fa4c6]
exit 1
It is Julia-version-sensitive — ABM's Core group passes on Julia lts (1.10) in this same CI run and the failure appears only on Julia 1 (1.12) — which points at Julia 1.12's stricter binding-at-import rules rather than any repo commit. OrdinaryDiffEqLowOrderRK imports u_cache/du_cache from SciMLBase correctly at src/OrdinaryDiffEqLowOrderRK.jl:20, and OrdinaryDiffEqCore neither imports nor exports those names, so something is emitting an export/GlobalRef against the wrong module. Handed off for its own investigation; it needs a separate fix and is out of scope here.
I have not re-run the remaining test matrices to completion — they were still in flight. If anything fails beyond the four above, treat it as unexplained and worth a look rather than assuming it is also pre-existing.
CI independently reproduces the ExplicitImports resultThe first touched sublibrary whose QA lane finished on CI is OrdinaryDiffEqVerner, and it matches my local before/after exactly:
Two corroborating data points that this red-lane behaviour is repo-wide and not introduced here: One correction to my earlier triage comment: The remaining QA lanes were still in flight. |
CI-side confirmation: ExplicitImports results for every completed QA laneThe QA lanes have now run on CI. Extracting the
Eight of the touched sublibraries reach Controls — untouched packages, same red lane:
Every one of these fails its lane too, all on One upgrade over the description: AdamsBashforthMoulton's lane now reaches Aqua on CI and reports |
Correction: SDIRK's
|
Please ignore until reviewed by @ChrisRackauckas.
What changed and why
full_cacheandresize_non_user_cache!are defined in andexported by SciMLBase (src/integrator_interface.jl:147/:162;exportblock atsrc/SciMLBase.jl:2077/:2079);AutoDePSpecializeis defined in SciMLBase and declaredpublicthere (src/SciMLBase.jl:2234, public since 3.39.0); theSciMLOperatorsmodule name is owned by SciMLOperators.OrdinaryDiffEqCore— and, forSciMLOperators, SciMLBase — merely re-exports these. 14 sublibraries reached them through the re-exporter, which fails ExplicitImports' owner/public checks in every QA lane that runs them.This applies the same transformation #4167 did for
OrdinaryDiffEqBDF: move the name into theimport SciMLBase:list, and changeOrdinaryDiffEqCore.AutoDePSpecializetoSciMLBase.AutoDePSpecialize. Import lines only — no behaviour change, no new dependency, no compat bump. 44 added lines, 41 removed, 0 comment lines, 14 files.Branched off
master, not off #4167 — the two touch disjoint files, so there is no ordering requirement and either can merge first.Affected sublibraries (re-derived, not inherited)
I re-derived the list by parsing every
lib/OrdinaryDiffEq*/src/<Name>.jlimport block rather than trusting a handed-down list, then cross-checked each name's owner and publicness against SciMLBase 3.39.0's sources. Result: 14 siblings besides BDF, and 12 of them have a QA lane withexplicit_imports = true.full_cacheAutoDePSpecializeSciMLOperatorsresize_non_user_cache!Two names beyond
full_cache/AutoDePSpecializeturned up, and both are included because without them the affected lane cannot go green:resize_non_user_cache!in Rosenbrock — SciMLBase-owned,exported, imported fromOrdinaryDiffEqCore, and absent from Rosenbrock's ignore list. Confirmed by the baseline lane, which names it alongsidefull_cache.SciMLOperatorsin Linear — owned by SciMLOperators, reached viausing SciMLBase: SciMLBase, SciMLOperators, SplitFunction. Routed through the existingusing SciMLOperators:line instead. This one is worth flagging: SciMLBase 3.40.0 deliberately dropped@reexport using SciMLOperatorsin SciML/SciMLBase#1472 ("Make strict QA owner-contract clean"), so this PR moves OrdinaryDiffEqLinear in the same direction SciMLBase itself went.MultirateandNewmarkhave atest/qa/directory containing onlyallocation_tests.jl— noqa.jl, hence no ExplicitImports checks. They are fixed at the source level; there is no lane to show a before/after on, so their evidence is a cleanGROUP=ALLrun instead.Compat and dependency verification
All 14 already have
SciMLBasein[deps]and already pinSciMLBase = "3.39", soAutoDePSpecializeis public at the pinned floor everywhere. No compat bump and no new dependency were needed. Checked mechanically:Verification
Julia 1.12.6. Baselines were run against a separate clean checkout of unmodified
master(7b1ad0368), so before/after are independent trees rather than a stash.ExplicitImports before → after — all 12 lanes run
rollupis theExplicitImportstestset line (pass [fail] [error] total). "target" counts the four checks this PR addresses:all_explicit_imports_via_owners,all_explicit_imports_are_public,all_qualified_accesses_via_owners,all_qualified_accesses_are_public.4 2 6full_cache6 64 2 6full_cache6 64 2 6full_cache6 64 2 6SciMLOperators,full_cache6 62 4 6AutoDePSpecialize,full_cache6 64 2 6full_cache6 62 4 6AutoDePSpecialize,full_cache6 64 2 6full_cache6 62 4 6AutoDePSpecialize,full_cache6 62 4 6AutoDePSpecialize,full_cache6 61 5 6AutoDePSpecialize,full_cache,resize_non_user_cache!, +Cartesian,lorenz_pref,lorenz_pref_params4 2 62 4 6AutoDePSpecialize,full_cache, +lorenz_pref,lorenz_pref_params,strip_cache5 1 6Ten of twelve reach 4/4. Rosenbrock and SDIRK reach 3/4, not 4/4 — read that carefully rather than assuming a clean sweep. In both,
all_qualified_accesses_are_publicstays red on names this PR does not touch and which are simply missing from those two packages' own ignore lists:lorenz_pref/lorenz_pref_params(OrdinaryDiffEqCore precompile-workload internals — note Tsit5'sqa.jldoes ignore them), plusCartesian(aBaseinternal, in Rosenbrock) andstrip_cache(in SDIRK). Rosenbrock additionally keeps a pre-existingno_stale_explicit_importserror onWOperatorand_reshape. Per the brief I did not add ignore entries to paper over any of these; they want their own PR (either make the name public at the owner, or add a documented ignore).The exact baseline diagnostic, e.g. for Tsit5:
Aqua totals, and nothing else moved
Whole-Aqua counts, showing the errors clearing while Allocation and JET stay byte-identical:
1 1→1 11 pass 2 broken→1 pass 2 broken4 4→4 41 1→1 138 38→38 381 1→1 117 broken→17 broken1 1→1 116 pass 2 broken→16 pass 2 broken1 pass 22 broken→1 pass 22 broken13 broken→13 broken27 27→27 278 8→8 81 1→1 1Every row's total is 20. The surviving
1 failin all of them is the docstring issue described below.The QA lanes remain RED overall — for reasons that are not this PR
public API has docstrings,Evaluated: isempty([:SciMLBase])fails in every one of these lanes, identically before and after. Knownmasterfailure, fixed in Exempt reexported external modules from the API docstring check SciMLTesting.jl#45. Do not read a red QA lane on this PR as a regression — the signal is the ExplicitImports subtree and the Aqua error column.AllocCheck aborts stopped the QA lane before Aqua in AdamsBashforthMoulton, ExponentialRK and Linear when I ran it. A parallel investigation established the root cause and it is not a code regression: a bare
Pkg.test()uses--check-bounds=yes, while SciML's Sublibrary CI passes--check-bounds=auto, and the AB3 allocation sites only survive under=yes. CI has never seen these. See AllocCheck allocation testsets disagree between CI (--check-bounds=auto) and local Pkg.test() (--check-bounds=yes) #4172 and the fix in Keep AB3's history-slot updates allocation-free (fixes the local ABM GROUP=QA failure) #4171.UndefVarError: MatrixOperatorfailsGROUP=Corein Linear and ExponentialRK, on clean master too. Cause: SciMLBase 3.40.0 dropped@reexport using SciMLOperators(SciML/SciMLBase#1472); those test files useMatrixOperatorwithout importing it. Fix in Import SciMLOperators names explicitly in tests (SciMLBase 3.40 dropped the re-export) #4170.Where an abort hid the Aqua testset, I obtained the ExplicitImports result by running that sublibrary's own
test/qa/qa.jldirectly in its QA env — the samerun_qacall with the sameei_kwargs, just without the two preceding testsets. This harness was cross-validated against the full lane on four sublibraries where both reached Aqua (HighOrderRK4 2 6, LowStorageRK2 4 6, RKN4 2 6, SSPRK2 4 6) and agreed exactly.Main test groups
GROUP=ALL(Core + QA) on the branch:2 broken.2 broken; JET1 1.1 fail, 12 broken, byte-identical to master (issue AllocCheck allocation testsets disagree between CI (--check-bounds=auto) and local Pkg.test() (--check-bounds=yes) #4172 above).Convergence102 pass / 2 broken on the master baseline (49 minutes); the branch's own SDIRK convergence run did not finish before I stopped the batch, see below.Multirate and Newmark are precisely the two sublibraries with no ExplicitImports lane, so their clean exit 0 is the evidence that their import move is sound.
Formatting / spelling
Runic.main(["--check", "--diff", <the 14 files>])→ exit 0 locally (Runic 1.7.0).typos <the 14 files>→ exit 0 locally (typos-cli 1.47.0, honouring the repo.typos.toml).CI agrees on the rebased head:
format-checksuccess,Runic Suggestionssuccess,Spell Checksuccess,Downgradesuccess,Documentationsuccess. The remaining test matrices were still running when I wrote this.What I did NOT verify
GROUP=ALLfunctional runs for Rosenbrock and SDIRK on the branch. Their convergence suites run ~50 minutes each and I stopped the batch before they completed; their ExplicitImports numbers above come from theqa.jlharness. RKN, SSPRK, StabilizedRK, Verner, LowStorageRK completed theirGROUP=ALLlanes (Aqua table above), but I have not tabulated their functional testset counts here.GROUP=GPU; not run, no GPU on this machine.Docs build— now covered: CI'sDocumentationworkflow passes on the rebased head. (The pre-rebase run failed onCannot resolve @ref for DummyControllerCacheindocs/src/devtools/internals/public_api.md, which is the pre-existing failure Fix docs build: unlink the unresolvable DummyControllerCache @ref #4164 fixed on master; the rebase picked that fix up.)julia pre— the QA lanes are gated onisempty(VERSION.prerelease)anyway — and the other allowed-to-fail jobs.Worth pushing back on
full_cacheis imported but never used outside the module header in 11 of the 14 (only Rosenbrock and SDIRK define methods on it). Deleting the import would arguably be cleaner than moving it; I moved it to stay consistent with the merged BDF change in Access full_cache and AutoDePSpecialize from SciMLBase in OrdinaryDiffEqBDF #4167. Say the word and I will delete instead.SciMLOperators(Linear) andresize_non_user_cache!(Rosenbrock) are scope beyond the two names in the original report. Same defect class, and the affected lanes cannot go green without them, but it is a judgement call.alg_order,isadaptive,initialize!,SplitFunctionreached viaOrdinaryDiffEqCore). None of those four has an ExplicitImports lane, so a fix there is unverifiable by CI and belongs in its own PR.all_qualified_accesses_are_publicfailures (lorenz_pref,lorenz_pref_params,Cartesian,strip_cache) are left red on purpose rather than silenced with ignore entries.🤖 Generated with Claude Code
https://claude.ai/code/session_01F75XsVeZ94QH3PmCuq6QUF