From f7e952e0d9aeb44835fb0fde352d98cae84adf4d Mon Sep 17 00:00:00 2001
From: Aaron Trowbridge <aaron.j.trowbridge@gmail.com>
Date: Wed, 8 Jul 2026 15:30:38 -0400
Subject: [PATCH] warn on inert PiccoloOptions fields (rollout_integrator /
 geodesic / zero_initial_and_final_derivative)

T0 defect from the 2026-07-08 API-ergonomics audit (spec spec-20260708-140523,
armonissima#33). Three PiccoloOptions fields are stored and documented as
load-bearing but never consulted anywhere in Piccolo (verified by whole-tree
grep): `rollout_integrator` (rollout algorithms are actually chosen by
`default_algorithm(system)`), `geodesic`, and `zero_initial_and_final_derivative`.

Setting any of them to a non-default value now emits a warning naming the real
mechanism / the removal (v2.0); the default path stays silent. Docstrings mark
all three DEPRECATED. Fields retained until v2.0 for source compatibility.

Visible @warn rather than Base.depwarn: Julia's default --depwarn=no hides
depwarn from interactive users, and the point is that a silently-ignored option
must not stay silent. Added a testitem asserting each warns on non-default and
that defaults / genuinely-consulted fields (bound_state) stay silent; required
`using TestItems` in the Options submodule (siblings already import it).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 src/control/options.jl | 47 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 44 insertions(+), 3 deletions(-)

diff --git a/src/control/options.jl b/src/control/options.jl
index e3c923e7..33a9d2df 100644
--- a/src/control/options.jl
+++ b/src/control/options.jl
@@ -5,6 +5,7 @@ export display_level
 export DISPLAY_SILENT, DISPLAY_COMPACT, DISPLAY_STANDARD, DISPLAY_DETAILED
 
 using ExponentialAction
+using TestItems
 
 
 # TODO: Add duration and symmetry options
@@ -66,9 +67,13 @@ Options for the Piccolo quantum optimal control library.
   `Δt_bounds`, letting the optimizer choose a non-uniform grid (concentrating
   samples where the pulse is changing fast). Set to `true` to force uniform
   spacing — useful when the target hardware has a fixed sample-rate.
-- `rollout_integrator::Function = expv`: Integrator to use for rollout
-- `geodesic = true`: Use the geodesic to initialize the optimization.
-- `zero_initial_and_final_derivative::Bool=false`: Zero the initial and final control pulse derivatives.
+- `rollout_integrator::Function = expv`: **DEPRECATED (removed in v2.0)** — stored
+  but never consulted; rollout algorithms are chosen by `default_algorithm(system)`.
+  A non-default value warns.
+- `geodesic = true`: **DEPRECATED (removed in v2.0)** — stored but never consulted
+  (geodesic initialization is not wired to this flag). Setting `false` warns.
+- `zero_initial_and_final_derivative::Bool=false`: **DEPRECATED (removed in v2.0)** —
+  stored but never consulted. Setting `true` warns.
 - `complex_control_norm_constraint_name::Union{Nothing, Symbol} = nothing`: Name of the complex control norm constraint.
 - `complex_control_norm_constraint_radius::Float64 = 1.0`: Radius of the complex control norm constraint.
 - `bound_state::Bool = true`: Keep the default [-1, 1] box bounds on each component
@@ -130,6 +135,23 @@ function PiccoloOptions(;
     end
     display === nothing && (display = :standard)
     display_level(display)  # validate
+    # DEPRECATED (v2.0 removal): these three fields are stored but never consulted
+    # anywhere in Piccolo (verified by whole-tree grep). Warn on a non-default value
+    # so it is not silently ignored. `rollout_integrator` is especially misleading —
+    # rollout algorithms are actually chosen by `default_algorithm(system)`.
+    rollout_integrator === expv || @warn(
+        "PiccoloOptions `rollout_integrator` is not consulted anywhere in Piccolo " *
+        "and will be removed in v2.0; rollout algorithms are chosen by " *
+        "`default_algorithm(system)`."
+    )
+    geodesic || @warn(
+        "PiccoloOptions `geodesic` is not consulted anywhere in Piccolo and will be " *
+        "removed in v2.0 (geodesic initialization is not wired to this flag)."
+    )
+    zero_initial_and_final_derivative && @warn(
+        "PiccoloOptions `zero_initial_and_final_derivative` is not consulted anywhere " *
+        "in Piccolo and will be removed in v2.0."
+    )
     return PiccoloOptions(
         display,
         timesteps_all_equal,
@@ -146,4 +168,23 @@ function PiccoloOptions(;
     )
 end
 
+@testitem "PiccoloOptions warns on deprecated inert fields" begin
+    using Piccolo
+
+    # `Logging` is not a Piccolo dependency; use the LogLevel from Base.CoreLogging
+    # (always available) so `@test_logs min_level=` filters to warnings-and-above.
+    # Defaults, and consulted fields, produce no warning.
+    @test_logs min_level = Base.CoreLogging.Warn PiccoloOptions()
+    @test_logs min_level = Base.CoreLogging.Warn PiccoloOptions(bound_state = false)
+
+    # The three inert fields each warn on a non-default value (removed in v2.0).
+    @test_logs (:warn, r"rollout_integrator") match_mode = :any PiccoloOptions(
+        rollout_integrator = identity,
+    )
+    @test_logs (:warn, r"geodesic") match_mode = :any PiccoloOptions(geodesic = false)
+    @test_logs (:warn, r"zero_initial_and_final_derivative") match_mode = :any PiccoloOptions(
+        zero_initial_and_final_derivative = true,
+    )
+end
+
 end