Skip to content

QA: run_qa v1.6 form + ExplicitImports#468

Merged
ChrisRackauckas merged 3 commits into
SciML:mainfrom
ChrisRackauckas-Claude:qa-run_qa-v1.6
Jul 3, 2026
Merged

QA: run_qa v1.6 form + ExplicitImports#468
ChrisRackauckas merged 3 commits into
SciML:mainfrom
ChrisRackauckas-Claude:qa-run_qa-v1.6

Conversation

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Contributor

Ignore until reviewed by @ChrisRackauckas. Draft, part of the SciML run_qa v1.6 / ExplicitImports rollout.

What

Brings this repo's QA onto SciMLTesting.run_qa v1.6 form with ExplicitImports enabled, replacing the hand-rolled test/qa/aqua.jl Aqua body.

  • New test/qa/Project.toml sub-env (Aqua + SciMLTesting "1.6" + SafeTestsets + Test, package via [sources]) so the QA group is isolated, matching the SciMLTesting folder model used by sibling repos. The QA group is discovered by run_tests() via test_groups.toml (unchanged).
  • test/qa/qa.jl now calls run_qa(ModelingToolkitStandardLibrary; explicit_imports = true, ...). Aqua + ExplicitImports come from SciMLTesting's own deps. JET is not run (the prior QA did not run JET). The one original non-broken Aqua tweak (ambiguities recursive = false, genuinely needed — recursive ambiguities fails in the dep tree) is preserved via aqua_kwargs.
  • Dropped the now-stale Aqua from the root [extras]/[targets].test/[compat] (Aqua moved into the QA sub-env). ExplicitImports stays transitive via SciMLTesting (not added as a direct dep).

ExplicitImports findings (6 checks, vs released SciMLTesting 1.6.0)

Check Outcome
no_stale_explicit_imports FIXED — removed genuinely unused imports
no_implicit_imports BROKEN (ei_broken) — tracked in #467
all_explicit_imports_via_owners PASS (ignore unwrap)
all_qualified_accesses_via_owners PASS
all_qualified_accesses_are_public PASS (ignores documented)
all_explicit_imports_are_public PASS (ignores documented)

Fixed (stale imports removed): RealOutput in Electrical, Hydraulic.IsothermalCompressible, Mechanical.TranslationalModelica; getdefault and using IfElse: ifelse in Mechanical.Translational.

Ignored (other packages' non-public names; go public as base libs release):

  • explicit-imports: unwrap (Symbolics, owned by SymbolicUtils), ifelse (IfElse), getdefault (ModelingToolkitBase)
  • qualified-accesses: ifelse (IfElse), SConst (Symbolics), depwarn (Base), isvariable/t_nounits (ModelingToolkitBase)

Broken (tracked): no_implicit_imports — the component submodules using ModelingToolkitBase, Symbolics, IfElse and rely on their exported names/macros (@component, @named, @variables, System, Equation, Flow, ...). Making all of these explicit is a large, mechanical, per-submodule refactor tracked in #467; kept as ei_broken = (:no_implicit_imports,) (auto-flags an Unexpected Pass once fixed).

Verification

Ran the QA group locally on Julia 1.10 with SciMLTesting 1.6.0 resolved from the registry (no dev-from-branch):

Test Summary: | Pass  Broken  Total
QA/qa.jl      |   16       1     17

0 Fail, 0 Error. Aqua sub-checks (ambiguities[recursive=false], unbound args, undefined exports, project_extras, stale_deps, deps_compat, piracy, persistent_tasks) all pass; the 5 enabled EI checks pass, no_implicit_imports is Broken.

🤖 Generated with Claude Code

ChrisRackauckas and others added 2 commits June 25, 2026 08:12
Convert the hand-rolled test/qa Aqua body to SciMLTesting's run_qa v1.6
declarative form and enable ExplicitImports (explicit_imports = true).

- Add a test/qa/Project.toml sub-env (Aqua + SciMLTesting "1.6" + SafeTestsets
  + Test, package via [sources]) so the QA group is isolated, matching the
  SciMLTesting folder model used by sibling repos.
- qa.jl now calls run_qa(ModelingToolkitStandardLibrary; explicit_imports = true,
  ...). Aqua + ExplicitImports come from SciMLTesting's own deps. JET is not run
  (the prior QA did not run JET). The original non-broken Aqua tweak
  (ambiguities recursive = false) is preserved via aqua_kwargs.
- ExplicitImports findings (vs released SciMLTesting 1.6.0):
  * no_stale_explicit_imports: FIXED by removing genuinely unused imports
    (RealOutput in Electrical / IsothermalCompressible / TranslationalModelica;
    getdefault + IfElse.ifelse in Translational).
  * all_explicit_imports_via_owners / *_are_public / all_qualified_accesses_are_public:
    ignore other packages' non-public names (unwrap<-Symbolics, ifelse<-IfElse,
    getdefault/isvariable/t_nounits<-ModelingToolkitBase, SConst<-Symbolics,
    depwarn<-Base). Documented per source.
  * no_implicit_imports: many submodules `using ModelingToolkitBase, Symbolics,
    IfElse` for exported names/macros; mass-explicit refactor is large and tracked
    in SciML#467, kept as ei_broken = (:no_implicit_imports,).
- Drop now-stale Aqua from the root [extras]/[targets].test/[compat] (Aqua moved
  to the QA sub-env; ExplicitImports stays transitive via SciMLTesting).

Verified locally on Julia 1.10 with SciMLTesting 1.6.0 from the registry:
QA group = 16 Pass, 1 Broken, 0 Fail, 0 Error.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Downgrade CI lane precompile-failed with a `CommonSolve.init` method
ambiguity between DiffEqBase's `init(::Union{AbstractDEProblem,
NonlinearProblem}, args...)` and NonlinearSolveBase's
`init(::AbstractNonlinearProblem, ::AbstractNonlinearTerminationMode, du, u,
...)`, surfaced while precompiling NonlinearSolveFirstOrder / OrdinaryDiffEq*.

DiffEqBase 6.190.0 (commit "remove NonlinearSolve things") dropped
`NonlinearProblem` from that `init` signature, removing the ambiguity. The
old floor 6.189.1 still carried the colliding method, so downgrade resolved
into the ambiguous state. Bump the floor to 6.190.

Verified on Julia 1.10: with DiffEqBase 6.190.0 and the CI-downgrade-resolved
solver stack (NonlinearSolveFirstOrder 1.7.0, NonlinearSolveBase 1.14.0,
SciMLBase 2.125.0, OrdinaryDiffEq 6.102.1, OrdinaryDiffEqBDF/SDIRK/Default/
NonlinearSolve at floors), all 51 packages precompile cleanly (the 5 that
failed in CI now pass). 6.190.0 requires SciMLBase >= 2.115.0, satisfied by
the resolved 2.125.0.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ChrisRackauckas-Claude ChrisRackauckas-Claude marked this pull request as ready for review June 29, 2026 09:51
Make every submodule's `using ModelingToolkitBase/Symbolics/IfElse` explicit so
the package no longer relies on implicit imports, then drop the corresponding
ei exceptions from the QA configuration.

Source changes (per-module explicit name lists derived from
ExplicitImports.print_explicit_imports on Julia 1.12):
- Each component submodule now imports exactly the macros/types it uses
  (e.g. @component, @connector, @nAmed, @parameters, @unpack, @variables,
  System, Equation, Flow, compose, connect, extend, ParentScope,
  domain_connect, @register_symbolic, @register_derivative, Differential),
  plus the bare module names that are accessed qualified.
- Blocks/sources.jl: `using DiffEqBase`/`using PreallocationTools` made explicit
  (DiffCache, GeneralLazyBufferCache, get_tmp); DiffEqBase had no used names so
  only its module name is imported.
- The thin wrapper modules (Magnetic, Mechanical, Hydraulic) only reference the
  ModelingToolkitBase module name, so they import just that.

qa.jl exceptions removed (verified now unnecessary against released
SciMLTesting 1.7.0 / SciMLBase 3.30 / ModelingToolkitBase 1.48 / Symbolics 7.29):
- ei_broken = (:no_implicit_imports,)  -> all 16 modules now pass the check.
- all_explicit_imports_via_owners ignore (:unwrap) -> passes with no ignore.
- all_explicit_imports_are_public: dropped :unwrap (public in Symbolics) and
  :getdefault (public in ModelingToolkitBase); only :ifelse remains.
- all_qualified_accesses_are_public: dropped :t_nounits (public) and :depwarn
  (no longer flagged); only :ifelse, :SConst, :isvariable remain.

Remaining ignores are irreducible (genuinely non-public upstream names):
ifelse (IfElse's sole export, not declared public), SConst (Symbolics-internal,
used in a @register_derivative), isvariable (ModelingToolkitBase-internal).

Verified on Julia 1.12.6: full QA group green via run_qa (17 Pass / 0 Broken,
was 16 Pass / 1 Broken), and a smoke test constructs one component from every
affected module successfully (Blocks.Sine/DeadZone, Electrical.Pin,
IsothermalCompressible.Cap, FluxTubes.Ground, etc.). Runic-formatted.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ChrisRackauckas ChrisRackauckas merged commit 776d7c7 into SciML:main Jul 3, 2026
5 of 11 checks passed
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.

2 participants