Repository navigation
Fix QA: approve the limited/constraints_to_penalties API additions - #5116
Merged
ChrisRackauckas merged 1 commit intoSep 12, 2026
Merged
ChrisRackauckas merged 1 commit into
ChrisRackauckas merged 1 commit into
Conversation
Three recently merged PRs widened ModelingToolkit's surface without updating the two version-controlled QA audit lists in `test/qa/qa.jl`, which is what those lists exist to catch: - SciML#4774 (`limited` operator) exports `limited`, `limitnew`, `limitold` from ModelingToolkitBase and reaches for `MTKBase.LimitedCtx`, `MTKBase.apply_limited_lowering` and `MTKBase.attach_stage_limiters` from the root package. - SciML#5111 exports `constraints_to_penalties` from ModelingToolkitBase. The four exported names are documented public API of ModelingToolkitBase with rendered `@docs` entries (`docs/src/basics/Limiting.md`, `docs/src/API/problems.md`), so they are added to `REEXPORTED_API`. The three qualified accesses are MTK/MTKBase shared implementation seams -- a metadata registry key and two lowering entry points populated by the compiler, not anything a user sets -- so they join `lower_homotopy` and `has_any_homotopy`, the directly analogous names from the `homotopy` operator, in `NONPUBLIC_QUALIFIED_ACCESSES`. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Agent-Harness: Claude Code 2.0.14 Agent-Model: claude-opus-5[1m] Agent-Session: https://claude.ai/code/session_01FeJYXni9MkJhPFA9yxYvfn Claude-Session: https://claude.ai/code/session_01FeJYXni9MkJhPFA9yxYvfn
ChrisRackauckas
marked this pull request as ready for review
September 12, 2026 10:46
This was referenced Sep 12, 2026
ChrisRackauckas
added a commit
that referenced
this pull request
Sep 13, 2026
- Add constraints_to_penalties: lower constraints to quadratic penalty costs (#5111) - Fix QA: approve the `limited`/`constraints_to_penalties` API additions (#5116) - Concretize initialization problems once at construction (#5062) - Make tovar public (#5120) - Fix SciCompDSL model macro hygiene (#5058) - Release ModelingToolkitBase 1.71.0 (#5126) - Forward ProblemTypeCtx metadata to OptimizationProblem (#5115) - Release ModelingToolkitBase 1.71.1 (#5127) Agent-Harness: Claude Code Agent-Model: claude-opus-5[1m] Claude-Session: https://claude.ai/code/session_014FEzNTLFutCmTEAZ3zBg5R Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Please ignore this PR until it has been reviewed by @ChrisRackauckas.
What changed and why
tests / QA (julia 1, ubuntu-latest)is red on master. Three recently merged PRs widened ModelingToolkit's API surface without updating the two version-controlled audit lists intest/qa/qa.jl— which is exactly what those lists exist to catch ("a dependency adding a new public name fails this test instead of silently widening ModelingToolkit's API").limitedoperator,3d9e44bab1) exportslimited,limitnew,limitoldfrom ModelingToolkitBase, and the root package reaches forMTKBase.LimitedCtx(src/systems/solver_nlprob.jl:48),MTKBase.apply_limited_lowering(src/systems/systems.jl:12) andMTKBase.attach_stage_limiters(src/systems/solver_nlprob.jl:121).887af5f455) exportsconstraints_to_penaltiesfrom ModelingToolkitBase.Two assertions fail, with two different root causes:
all_qualified_accesses_are_public—LimitedCtx,apply_limited_lowering,attach_stage_limiters. These are MTK↔MTKBase shared implementation seams: a metadata registry key the compiler populates (never something a user sets) and two lowering entry points. They are the direct analogues oflower_homotopyandhas_any_homotopyfrom thehomotopyoperator, which already sit inNONPUBLIC_QUALIFIED_ACCESSES, so they join them there. Declaring thempublicin ModelingToolkitBase would advertise compiler internals to end users, which the comment above that list explicitly rejects as the wrong fix for this group (tracked in QA: ModelingToolkit depends on ~140 non-public names of its dependencies #4882). Note the contrast withHomotopyCtx, which is@public+ documented: that one is a user-settable toggle,LimitedCtxis a generated registry.No unapproved public reexports—constraints_to_penalties,limited,limitnew,limitold. Different cause: these are genuinely new public, documented API of ModelingToolkitBase, with rendered@docsentries already in place (docs/src/basics/Limiting.mdlines 199-203 for the three limiting names,docs/src/API/problems.mdforconstraints_to_penalties). Approving them inREEXPORTED_APIis the intended action for that list. No docs changes are needed — thePublic API documentationcheck passes both before and after.Test-file-only change; no library code is touched.
Verification
All runs on this branch's checkout, Julia 1.12.7,
GROUP=QA julia --project -e 'using Pkg; Pkg.test()'.Failing before the fix (unmodified master,
887af5f455):Passing after the fix:
GROUP=InterfaceI(nothing else broke; the 3 broken are pre-existing@test_broken):Formatting / spelling, both clean:
Not verified
docs/, and no public API. The@docsentries for all four newly-approved names already exist on master and the QAPublic API documentationcheck passes, which is what would have caught a missing one.InterfaceII,Initialization,SymbolicIndexingInterface,Downstream,FMI,Extensions,Optimization) were not run. The change is confined totest/qa/qa.jl, which only the QA group loads.lts. Theltshalf has its own extra findings (see theIterators.mapnote in the file), but nothing in this diff is version-dependent.What a reviewer should push back on
Whether
LimitedCtx,apply_limited_loweringandattach_stage_limitersreally belong on the ignore list rather than being promoted to public API in ModelingToolkitBase. I chose the ignore list because it is this repo's documented, exhaustive convention for MTK↔MTKBase seams and because the homotopy operator's equivalents are handled that way — but that is a judgment call about intent, and the author of #4774 may have meantLimitedCtxto be user-facing the wayHomotopyCtxis. If so, the fix is a@public+ docstring +@docsentry in ModelingToolkitBase instead, and I'll redo it that way.🤖 Generated with Claude Code (model: claude-opus-5[1m])
https://claude.ai/code/session_01FeJYXni9MkJhPFA9yxYvfn