From 40a007aca55e4956505028a17e53b2fd012094fd Mon Sep 17 00:00:00 2001 From: Olivier Cots Date: Wed, 29 Jul 2026 23:03:37 +0200 Subject: [PATCH] feat(strategies): non-throwing parameter(T, default) accessor Strategies.parameter(T) throws NotImplemented by default, same as id and metadata. But "no parameter" is a legitimate, already-established answer (nothing) for parameter specifically, not an oversight the way a missing id or metadata would be. The real gap is callers that query parameter on a strategy type they did not author and cannot guarantee has overridden the (optional) contract - they had to catch NotImplemented themselves, as OptimalControl currently does by hand in helpers/print.jl. Adds parameter(strategy_type, default), the non-throwing counterpart to parameter(strategy_type) - the same relationship as get(dict, key, default) to dict[key]. default is only substituted when the underlying call throws NotImplemented; a strategy that legitimately implements parameter and returns nothing still returns nothing, not default, and any other exception still propagates. Implemented as a single get-style 2-arg method rather than the issue's alternative keyword-based shape (parameter(T; strict=false)): Julia resolves keyword arguments only after dispatching on positional types, against whichever method that dispatch selects. Every currently-implemented strategy overrides parameter with its own plain 1-arg method with no keyword declared, so the keyword form would throw "no keyword argument strict" for every strategy that already implements the contract correctly, and only work for the ones that don't. The 2-arg method has no such issue: it wraps a call to parameter(T) internally and works uniformly regardless of which concrete method that inner call resolves to. Fixes control-toolbox/CTBase.jl#518 Co-Authored-By: Claude Sonnet 5 --- src/Strategies/contract/abstract_strategy.jl | 52 +++++++++++++++++++ .../strategies/test_abstract_strategy.jl | 38 ++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/src/Strategies/contract/abstract_strategy.jl b/src/Strategies/contract/abstract_strategy.jl index 09a992eb..7f4c1a4c 100644 --- a/src/Strategies/contract/abstract_strategy.jl +++ b/src/Strategies/contract/abstract_strategy.jl @@ -226,6 +226,9 @@ Strategies.parameter(::Type{<:MyStrategy}) = nothing Strategies.parameter(::Type{<:MyStrategy{P}}) where {P<:AbstractStrategyParameter} = P ``` +A non-throwing 2-arg variant, `parameter(strategy_type, default)`, is also available for +callers that cannot guarantee the type they're querying implements this contract — see below. + See also: [`CTBase.Strategies.default_parameter`](@ref), [`CTBase.Strategies.AbstractStrategyParameter`](@ref) """ function parameter end @@ -304,6 +307,55 @@ function parameter(::Type{T}) where {T<:AbstractStrategy} ) end +""" +$(TYPEDSIGNATURES) + +Return the strategy parameter type for a concrete strategy type, or `default` if the +strategy type does not implement the `parameter` contract at all. + +This is the non-throwing counterpart to `parameter(strategy_type)` — the same relationship +as `get(dict, key, default)` to `dict[key]`. Use it when querying a strategy type you did +not author and cannot guarantee has overridden the (optional) `parameter` method; use the +1-arg form when the type is known to implement the contract and a missing implementation +should be a loud error. + +Note this is different from a strategy that *does* implement `parameter` and legitimately +returns `nothing` (the documented, established answer for "no parameter", e.g. `Ipopt`) — +that `nothing` is returned as-is, not replaced by `default`. `default` is only substituted +when the underlying call throws `Exceptions.NotImplemented`; any other exception still +propagates. + +# Arguments +- `strategy_type::Type{<:AbstractStrategy}`: The strategy type +- `default`: Value to return if `strategy_type` does not implement `parameter` + +# Returns +- Whatever `parameter(strategy_type)` returns, or `default` if that call throws + `Exceptions.NotImplemented` + +# Example +```julia-repl +julia> parameter(SomeThirdPartyStrategy, nothing) # doesn't throw even if unimplemented +nothing + +julia> parameter(Ipopt, :fallback) # Ipopt implements parameter -> nothing, not :fallback +nothing + +julia> parameter(MadNLP{CPU}, :fallback) +CPU +``` + +See also: [`CTBase.Strategies.parameter`](@ref), [`CTBase.Strategies.default_parameter`](@ref) +""" +function parameter(strategy_type::Type{<:AbstractStrategy}, default) + return try + parameter(strategy_type) + catch e + e isa Exceptions.NotImplemented || rethrow() + default + end +end + """ Default implementation for `options(strategy::T)` with flexible field access. diff --git a/test/suite/strategies/test_abstract_strategy.jl b/test/suite/strategies/test_abstract_strategy.jl index c2433337..3826f74b 100644 --- a/test/suite/strategies/test_abstract_strategy.jl +++ b/test/suite/strategies/test_abstract_strategy.jl @@ -52,6 +52,17 @@ Strategies.options(strategy::FakeStrategy) = strategy.options # Additional test struct for error handling struct UnimplementedStrategy <: Strategies.AbstractStrategy end +# Parameterized fake, to prove a real value passes through the 2-arg parameter() unchanged. +struct FakeStrategyParam{P<:Strategies.AbstractStrategyParameter} <: Strategies.AbstractStrategy end +Strategies.parameter(::Type{<:FakeStrategyParam{P}}) where {P} = P + +# Overrides parameter but throws something OTHER than NotImplemented — proves the 2-arg +# wrapper only swallows NotImplemented and rethrows everything else. +struct FakeStrategyBadParameter <: Strategies.AbstractStrategy end +function Strategies.parameter(::Type{<:FakeStrategyBadParameter}) + return throw(Exceptions.IncorrectArgument("deliberately not NotImplemented")) +end + # Fake strategy with description for testing multi-line display struct FakeStrategyWithDescription <: Strategies.AbstractStrategy options::Strategies.StrategyOptions @@ -153,6 +164,33 @@ function test_abstract_strategy() ) end + Test.@testset "parameter(T, default) - non-throwing accessor" begin + # Falls back to default when parameter() is not implemented at all. + Test.@test Strategies.parameter(UnimplementedStrategy, nothing) === nothing + Test.@test Strategies.parameter(UnimplementedStrategy, :fallback) === + :fallback + + # A strategy that legitimately returns nothing is NOT conflated with + # "unimplemented": this just confirms the passthrough works when there is + # no exception at all. + Test.@test Strategies.parameter(FakeStrategy, :fallback) === nothing + + # A real parameter value passes through unchanged. + Test.@test Strategies.parameter(FakeStrategyParam{Strategies.CPU}, :fallback) === + Strategies.CPU + + # Non-NotImplemented errors still propagate — the wrapper is not a blanket + # catch-all. + Test.@test_throws Exceptions.IncorrectArgument Strategies.parameter( + FakeStrategyBadParameter, nothing + ) + + # 1-arg form is unchanged: still throws for a genuinely unimplemented strategy. + Test.@test_throws Exceptions.NotImplemented Strategies.parameter( + UnimplementedStrategy + ) + end + Test.@testset "Collection interface - getindex" begin # Use build_strategy_options to properly initialize alias_map fake_opts = Strategies.build_strategy_options(