feat: add JET/BenchmarkTools performance verification#373
Merged
Conversation
Mirrors the control-toolbox Handbook's performance-verification practice
(hot path vs. setup path) already deployed in CTBase:
- test/suite/meta/test_code_quality.jl (renamed from test_aqua.jl): keeps
Aqua checks and adds JET.@test_opt guards on hot-path calls (time-function
functors, dual-by-label functors, box-projection functor, component
accessors). A whole-package JET.test_package scan was deliberately not
wired in as a gate: CTModels' mutable PreModel builder has Union{T,Nothing}
fields by design, and report_package's union-split analysis flags ~40
unreachable branches there — setup-path noise, not hot-path regressions.
- test/suite/meta/test_performance.jl (new): BenchmarkTools.@ballocated
allocation guards — zero-overhead wrapper checks and zero-allocation reads.
- Test.@inferred guards added next to their fixtures in
test_components.jl, test_dual_model.jl and test_model.jl.
- docs/src/guide/performance.md (new): guide with live JET.@report_opt
checks executed at doc-build time, wired into the nav.
Also fixes a real gap surfaced while triaging the JET scan: PreModel was
missing control_dimension/variable_dimension methods (only state_dimension
existed), so Init code dispatching generically on Models.AbstractModel could
hit a MethodError if ever called with a PreModel.
Co-Authored-By: Claude Opus 5 <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
1 participant
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.
Summary
Brings CTModels.jl's performance-verification setup in line with the control-toolbox Handbook's practice (hot path vs. setup path), already deployed in CTBase.
test/suite/meta/test_code_quality.jl(renamed fromtest_aqua.jl): keeps Aqua checks, addsJET.@test_optguards on hot-path calls — time-function functors (ConstantInTime,CoercedTrajectory), dual-by-label functors (DualSlice,BoxDualDiff), the box-projection functor (BoxProjection), and component accessors.test/suite/meta/test_performance.jl(new):BenchmarkTools.@ballocatedallocation guards — zero-overhead wrapper checks (wrapper allocation == raw allocation) and zero-allocation reads.Test.@inferredguards added next to their existing fixtures intest_components.jl,test_dual_model.jl, andtest_model.jl.docs/src/guide/performance.md(new): guide page with liveJET.@report_optchecks executed at doc-build time, wired into the nav.Project.toml/docs/Project.toml: JET and BenchmarkTools added test-only (never[deps]).Design note: no whole-package
JET.test_packagegateA one-off
JET.report_packagescan found 46 "possible errors," all originating from the mutablePreModelbuilder, whose fields areUnion{T,Nothing}by design (filled in incrementally, validated with precondition checks before being read). JET's union-split analysis flags these as unreachable-in-practice branches — setup-path noise per the Handbook's own guidance, not hot-path regressions. Rather than hardening working setup-path code purely to satisfy the analyzer, or gating CI on known false positives, JET is applied precisely where it matters:@test_opton concrete hot-path calls.report_packageremains available as a documented, manual diagnostic.Side fix
Triaging the scan surfaced one real (if unreachable-via-the-public-API) gap:
PreModelwas missingcontrol_dimension/variable_dimensionmethods — onlystate_dimensionexisted. Added both, mirroringstate_dimension's pattern, soInitcode dispatching onModels.AbstractModeldoesn't hit a strayMethodErrorif ever called with aPreModel.Test plan
JET.report_package(CTModels)scan run and triaged by hand (46 findings, all setup-path false positives, zero in the hot path)test/suite/meta/test_code_quality.jl— 20/20 passing (Aqua +JET.@test_opt)test/suite/meta/test_performance.jl— 12/12 passing (@ballocatedguards)julia --project=docs docs/make.jl— build succeeds; all 4JET.@report_optblocks inperformance.mdrenderNo errors detected🤖 Generated with Claude Code