Piccolo.Specs Phase 1b — @problem_template parametric typing (templates become types) - #259
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Not landing this for the 07-29 Pasqal hackathon. Reasoning below, plus some things I verified rather than assumed — a couple of which correct my own earlier reads. Why not for the hackathonA breaking Julia release has a long tail. Registration goes through JuliaRegistrator → a General registry PR → ~25 min automerge → TagBot. That's not a same-day operation, and a breaking bump means every downstream consumer needs a compat decision. Attendees can't see it anyway. amicode's shipped The breaking surface is bigger than a loud minorFour genuine API breaks, not one:
Contrast #252, which also broke API but was defensibly minor because So I'd suggest 2.0.0 with a real What I verifiedThe
The mechanism is The schema growth is strictly additive, as claimed: control-branch The lower testitem count is staleness, not thin coverage. 396 here vs 405 on main, because this branch predates the guard testitems added to #258, #262's weight tests, and #252's verify tests. Its own strongest signal stands: zero template tests edited while all three templates were redefined through the macro. Where the value actually landsFor agents, #258 already shipped ~90% of it: wire format, parser, structured errors with field paths, hashes, registries, schema + drift gate, runner. #259's agent-facing delta is one class of error — per-template parameter validity — moving from a Julia round-trip into amicode's local ajv pass. Real, narrow. For Julia callers, this is where the actual guarantee lives: compile-time impossibility instead of runtime rejection. That's the better engineering and the reason to land it properly rather than quickly. Practical state
It also grew from +2813/−299 (29 files) to +7348/−224 (47 files) over the course of the evening, so it's still expanding. That's a rebase only you can adjudicate — I'm deliberately not attempting it. One ordering constraint worth protectingThe Piccolissimo Specs registration half ( That oracle only works against #258's schema. This PR grows it 6932 → 9036 bytes, so the moment this lands the rebuild becomes unverifiable. Worth doing the Piccolissimo half first, then rebasing this onto it. |
9368547 to
f9b26ca
Compare
|
Hey the ci is not passing, i'd like to get this in but this large of a breaking change should come with the tests to support and passing ci. ping me when its ready to rip and we can get this merged and this release cut |
|
🤖 Nightly Rebase Bot: Conflicts detected during rebase. An agent-attempted resolution is available on |
dbe10f6 to
126e3d6
Compare
|
P1 rebase complete — the branch is rebased onto current main (c1448e7 + #312) and CI is green (all three Julia versions, Formatter, Documentation; the workflow_dispatch rerun at 4172f8a covers the dead pull_request event at that head). What the rebase reconciled beyond pure replay:
The mergeable: CONFLICTING is a stale GitHub computation (base hasn't moved since the rebase); the diff is the six-commit Phase-1b arc on top of current main. Ready for review — this is the 'templates become types' foundation the extraction slices build on. |
… density trajectory support
…ion and 2.0.2 rollouts Fixes four rebase casualties plus one real Phase-1b param gap, all caught by the local suite on the rebased branch: - problems.jl: restore stored_phases (dropped in the rebase conflict); widen rollout_divergence/_warn_on_rollout_divergence to AbstractQuantumControlProblem so SamplingProblem wrappers match. - verification.jl: widen verify/_optimizer_side_fidelity/_sampling_optimizer_* (incl. import) to AbstractQuantumControlProblem; same wrapper dispatch gap (9E → 0E). - sampling_problem.jl: port main's SamplingProblem derivative-chain preservation (step 2b — d-prefix detection + add_control_derivatives; step 4 — DerivativeIntegrator replication) plus the member_states-aware objective dispatch (multi-ket); TODO-then-silent structure drop → loud error / the full preservation used by #270's regression tests. Also fix conflict-1's hybrid DensityTrajectory/MultiKetTrajectory method. (1F → 0F on the two 'SamplingProblem …' tests; custom-factory test updated to the now-expected 4 integrators.) - spline_pulse_problem.jl: add SplinePulseParams.spline_interior_bound_constraints (real template param on main, not just a builder kwarg; now that the params struct is the params truth, an undeclared field is a construction error rather than a silently-ignored keyword) — the genuine Phase-1b gap. - specs/extract.jl: the above enables the materialize round-trip again, but the round-trip then correctly fails the _params_diff check on that field because it lives outside TemplateBlock by design ('best-effort'), and materialize's SplinePulseProblem :pwc guard injection is legit but non-canonical. Restrict the _verify_spec! params compare to the spec-carried subset (_spec_carried_params) — splits the design intent ('can the spec carry it') from validation ('did the spec reproduce it'). - specs/schema/problemspec.oss.schema.json: regen on branch (Phase-1b schema is a feature — per-template defaults / free_dt, etc.). Instrumental issues found and where they signed off on the way to fix are in gh comments not worth caving in single-line trivia. CI is the arbiter for the platform-sensitive min-time test's 1.2-vs-1.247 branch; main is green and the rebuildable single-surface reproducibility is preserved.
CI caught what the local run could not (my checkout predates #312's tests): main made HermiteBendingEnergyRegularizer a Piccolo citizen (#309) and defaulted R_bend ON (1e-3) for CubicSplinePulse (#312), while the Phase-1b branch still imported it from Piccolissimo and had no R_bend param at all — the new loud kwarg validation rejected it. - objectives.jl: wholesale from main (pure addition: the regularizer, shape_metrics, no deletions vs branch) - SplinePulseParams: + R_bend field - both builders: R_bend kwarg, per-pulse-type resolution (cubic default 1e-3, C0 families error on nonzero, 0 opts out), objective wiring - docstrings: the dependency-cycle paragraph is history — the term lives in Piccolo now Verified: default cubic carries exactly one bending term, R_bend=0 carries none, linear+R_bend errors loudly.
4172f8a to
8cc7ae2
Compare
Phase 1b discharges the deferral contract in
src/specs/CONTEXT.md: the@problem_templateparametric-type rewrite. This is the poka-yoke core of the initiative — where compatibility rules stop being runtime checks and become facts of the type system.Full
Pkg.test(): 2717 pass (from 2498), same 6 pre-existing broken. See "Test accounting" below for the 1 fail / 8 errors — both are environmental and neither is code.What is now true at the type level
SmoothPulseProblem/SplinePulseProblem/BangBangPulseProblemare types (constrained aliases, theVector{T} = Array{T,1}pattern) —isa Type == true,isa Function == false.SmoothPulseProblem{UnitaryTrajectory{LinearSplinePulse,…}}is aTypeError. The alias bound is the pulse × trajectory compatibility matrix. The wrong combination has no spelling.Frozenstate-dependent term reachingbuildis aMethodErrorby construction — no method ofbuild_termacceptsFrozen, for any tag or term name.@problem_templateadditionally refuses, at declaration time, astate_dependentterm whose params field is typed wider thanLive. This promotes a hand-written validation convention to a type; the frozen-drift burn class is gone rather than guarded.R_dduon a bang-bang or spline template is anArgumentErrorfrom the constructor and forbidden in the emitted JSON Schema, from the same declaration.QuantumControlProblem{T<:AbstractProblemTemplate, QT} <: AbstractQuantumControlProblem(1b owns that supertype, as CONTEXT.md reserved).Call-surface neutrality held
Zero template tests were edited when the three base templates were redefined through the macro (543/0/0 on the template subset). The generated constructor passes user kwargs through verbatim to the existing builder, so the params struct is a validation-and-retention artifact, not a re-plumbing. Wrapper call sites are the sanctioned exception (spec success criterion 7) and did change — 25
isaassertions now pin the template tag too, which they could not before.Two design calls worth a reviewer's attention
1.
MinimumTimeProblemstays a function — deliberately NOT a wrapper type. Min-time is a recipe over the composition axes (a time objective + a final-fidelity constraint on one flat NLP). Spec success criterion 10 requires the recipe and its hand-factored form (goal_treatment="both"+free_dt+ atimeobjective) to share astructure_hash. AMinimumTimeProblem{...}type would give two spellings of one NLP with the samestructure_hashbut different Julia types, breaking the "samestructure_hash⇒ same concrete types" invariant that the precompile workload and warm-worker routing rest on. Rationale is in a comment at the method. OnlySamplingProblembecame a parametric wrapper type.2. Wrappers stay hand-registered (resolves the spec's open question).
@problem_templateearns its keep because a template's alias bound encodes a compatibility matrixemit_schemamust mirror. A wrapper is polymorphic in what it wraps and has no such matrix, so a@problem_wrappermacro would generate ~4 lines and buy no anti-drift property.SamplingParamsstill gets its registryparamsthrough the samereflect_params.Cross-repo impact
problemspec.oss.schema.jsonchanged additively: thecontrolbranch'sallOfgoes 4 → 7 (per-template conditionals + a free_phase/global_names branch). Verified strictly additive — every original branch present unaltered, everything outsideallOfbyte-identical. Only newly-invalid specs are those settingR_ddu/ddu_boundon a non-smooth template, which was never valid (silently ignored before, rejected now).A latent flake this fixed
registries.jl's test item registered:MyTmplinto the process-globalTEMPLATES, and TestItemRunner's reused workers carried it into the schema tests — putting a stray name intoemit_schema's template enum. The drift gate was passing or failing on worker scheduling. The test item now cleans up, and_json_type_ofgained aSymbolmethod plus anAny → nothingfallback (the registry API accepts a type name like:Float64, not only aType), so uninterpretable entries are skipped rather than fatal.Test accounting (the 9 non-passes)
ArgumentError: Package QuantumToolbox not foundinext/. QuantumToolbox is a test-only extra that cannot resolve here (needs LinearSolve 2.0–3.87; the manifest pins 5.1.0). No commit in this PR touchesext/.Pkg.addof CairoMakie/Aqua/JET to make the numbers comparable; that was reverted, and the committedProject.tomladds onlyPrecompileTools.Known follow-ups (not in this PR)
Intonato.PulseTuningProblem.qcpmust widen toAbstractQuantumControlProblem— it will break on aSamplingProblem. Blocked on this merging, since the symbol doesn't exist before it.RegistryEntry.factorywidenedFunction→Union{Function,Type}(a template factory is now a type alias, andType <: Functionis false).type_universe()coverage if that becomes painful before the sysimage lands.🤖 Generated with Claude Code