Skip to content

Commit 1aabf6b

Browse files
committed
bugfix + cleanup docstrings
1 parent 773cbe9 commit 1aabf6b

File tree

8 files changed

+55
-46
lines changed

8 files changed

+55
-46
lines changed

docs/src/custom_benchmarks.md

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,10 @@ problems to the benchmark suite or integrate their own domains.
99
## Type hierarchy
1010

1111
```
12-
AbstractStaticBenchmark
13-
└── AbstractStochasticBenchmark{exogenous}
14-
└── AbstractDynamicBenchmark{exogenous}
12+
AbstractBenchmark
13+
├── AbstractStaticBenchmark
14+
├── AbstractStochasticBenchmark{exogenous}
15+
└── AbstractDynamicBenchmark{exogenous}
1516
```
1617

1718
| Type | Use case |
@@ -70,7 +71,7 @@ generate_maximizer(bench::MyBenchmark)
7071

7172
```julia
7273
generate_baseline_policies(bench::MyBenchmark) -> collection of callables
73-
is_minimization_problem(bench::MyBenchmark) -> Bool # default: false (maximization)
74+
is_minimization_problem(bench::MyBenchmark) -> Bool # default: true (minimization)
7475
objective_value(bench::MyBenchmark, sample::DataSample, y) -> Real
7576
compute_gap(bench::MyBenchmark, dataset, model, maximizer) -> Float64
7677
has_visualization(bench::MyBenchmark) -> Bool # default: false; return true when plot methods are implemented/available
@@ -80,7 +81,7 @@ plot_solution(bench::MyBenchmark, sample::DataSample; kwargs...)
8081

8182
---
8283

83-
## `AbstractStochasticBenchmark{true}`: additional methods
84+
## `AbstractStochasticBenchmark{true}`
8485

8586
For stochastic benchmarks with exogenous uncertainty, implement:
8687

@@ -113,7 +114,7 @@ DataSample(; x=features, y=nothing,
113114

114115
---
115116

116-
## `AbstractDynamicBenchmark`: additional methods
117+
## `AbstractDynamicBenchmark`
117118

118119
Dynamic benchmarks extend stochastic ones with an environment-based rollout interface.
119120

docs/src/using_benchmarks.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,8 @@ dataset = generate_dataset(bench, 50; rng=rng)
150150
gap = compute_gap(bench, dataset, model, maximizer)
151151
```
152152

153-
# Objective value for a single decision
153+
Objective value for a single decision:
154+
154155
```julia
155156
obj = objective_value(bench, sample, y)
156157
```

src/DynamicAssortment/DynamicAssortment.jl

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -78,17 +78,6 @@ include("policies.jl")
7878
"""
7979
$TYPEDSIGNATURES
8080
81-
Outputs a data sample containing an [`Instance`](@ref).
82-
"""
83-
function Utils.generate_sample(
84-
b::DynamicAssortmentBenchmark, rng::AbstractRNG=MersenneTwister(0)
85-
)
86-
return DataSample(; instance=Instance(b, rng))
87-
end
88-
89-
"""
90-
$TYPEDSIGNATURES
91-
9281
Generates a statistical model for the dynamic assortment benchmark.
9382
The model is a small neural network with one hidden layer of size 5 and no activation function.
9483
"""

src/Utils/interface/abstract_benchmark.jl

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ Generate a single unlabeled [`DataSample`](@ref) (with `y=nothing`) for the benc
1313
function generate_instance(bench::AbstractBenchmark, rng::AbstractRNG; kwargs...)
1414
return error(
1515
"`generate_instance` is not implemented for $(typeof(bench)). " *
16-
"Implement `generate_instance(::$(typeof(bench)), rng; kwargs...) -> DataSample` " *
17-
"or override `generate_sample` directly.",
16+
"Implement `generate_instance(::$(typeof(bench)), rng; kwargs...) -> DataSample`. " *
17+
"For static benchmarks, you may also override `generate_sample` directly instead.",
1818
)
1919
end
2020

@@ -48,9 +48,11 @@ end
4848
generate_baseline_policies(::AbstractBenchmark) -> NamedTuple or Tuple
4949
5050
Return named baseline policies for the benchmark. Each policy is a callable.
51+
The calling convention matches the `target_policy` signature for the benchmark category:
5152
52-
- For static/stochastic benchmarks: signature `(sample) -> DataSample`.
53-
- For dynamic benchmarks: signature `(env) -> Vector{DataSample}` (full trajectory).
53+
- **Static:** `(sample) -> DataSample`
54+
- **Stochastic:** `(ctx_sample, scenarios) -> Vector{DataSample}`
55+
- **Dynamic:** `(env) -> Vector{DataSample}` (full trajectory rollout)
5456
"""
5557
function generate_baseline_policies end
5658

@@ -79,9 +81,13 @@ function plot_solution end
7981
"""
8082
objective_value(bench::AbstractBenchmark, sample::DataSample, y) -> Real
8183
82-
Compute the objective value of given solution `y` for a specific benchmark.
83-
Must be implemented by each concrete benchmark type. For stochastic benchmarks,
84-
an additional `scenario` argument is required.
84+
Compute the objective value of solution `y` for the benchmark instance encoded in `sample`.
85+
Must be implemented by each concrete [`AbstractStaticBenchmark`](@ref).
86+
87+
For stochastic benchmarks, implement the 4-arg form instead (see
88+
[`ExogenousStochasticBenchmark`](@ref)):
89+
90+
objective_value(bench, sample, y, scenario) -> Real
8591
"""
8692
function objective_value end
8793

src/Utils/interface/dynamic_benchmark.jl

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,8 @@
22
$TYPEDEF
33
44
Abstract type interface for multi-stage stochastic (dynamic) benchmark problems.
5-
6-
Extends [`AbstractStochasticBenchmark`](@ref). The `{exogenous}` parameter retains its
7-
meaning (whether uncertainty is independent of decisions).
5+
The `{exogenous}` parameter has the same meaning (whether uncertainty is independent
6+
of decisions) as in [`AbstractStochasticBenchmark`](@ref).
87
98
# Primary entry point
109
- [`generate_environments`](@ref)`(bench, n; rng)`: mandatory (or implement
@@ -35,14 +34,14 @@ is_endogenous(::AbstractDynamicBenchmark{exogenous}) where {exogenous} = !exogen
3534
"""
3635
$TYPEDSIGNATURES
3736
38-
Intercepts accidental calls to `generate_sample` on dynamic benchmarks and throws a
39-
descriptive error pointing at the correct entry point.
37+
Intercepts accidental calls to the default `compute_gap` on dynamic benchmarks and throws a
38+
descriptive error. Dynamic benchmarks do not have a generic single-sample gap computation;
39+
override `compute_gap` directly on the concrete type if needed.
4040
"""
41-
function generate_sample(bench::AbstractDynamicBenchmark, rng; kwargs...)
41+
function compute_gap(bench::AbstractDynamicBenchmark, args...; kwargs...)
4242
return error(
43-
"`generate_sample` is not supported for dynamic benchmarks ($(typeof(bench))). " *
44-
"Use `generate_environments` and " *
45-
"`generate_dataset(bench, environments; target_policy=...)` instead.",
43+
"`compute_gap` is not supported for dynamic benchmarks ($(typeof(bench))). " *
44+
"Override `compute_gap` on the concrete type with trajectory-based evaluation logic.",
4645
)
4746
end
4847

@@ -92,12 +91,11 @@ to obtain standard baseline callables (e.g. the anticipative solver).
9291
9392
# Keyword arguments
9493
- `target_policy`: **required** callable `(env) -> Vector{DataSample}`.
95-
- `seed`: passed to `MersenneTwister` when `rng` is not provided.
96-
- `rng`: random number generator.
9794
"""
9895
function generate_dataset(
9996
bench::ExogenousDynamicBenchmark, environments::AbstractVector; target_policy, kwargs...
10097
)
98+
isempty(environments) && return DataSample[]
10199
return reduce(vcat, (target_policy(env) for env in environments))
102100
end
103101

@@ -114,7 +112,7 @@ function generate_dataset(
114112
bench::ExogenousDynamicBenchmark, n::Int; target_policy, seed=nothing, kwargs...
115113
)
116114
environments = generate_environments(bench, n; seed)
117-
return generate_dataset(bench, environments; target_policy, seed, kwargs...)
115+
return generate_dataset(bench, environments; target_policy, kwargs...)
118116
end
119117

120118
"""

src/Utils/interface/static_benchmark.jl

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,12 @@ Also implement:
2121
2222
# Optional methods (defaults provided)
2323
- [`is_minimization_problem`](@ref): defaults to `true`
24-
- [`objective_value`](@ref): defaults to `dot(θ, y)`
2524
- [`compute_gap`](@ref): default implementation provided; override for custom evaluation
2625
- [`has_visualization`](@ref): defaults to `false`
2726
27+
# Mandatory methods (no default)
28+
- [`objective_value`](@ref)`(bench, sample, y)`: must be implemented by every static benchmark
29+
2830
# Optional methods (no default, require `Plots` to be loaded)
2931
- [`plot_instance`](@ref), [`plot_solution`](@ref)
3032
- [`generate_baseline_policies`](@ref)

src/Utils/interface/stochastic_benchmark.jl

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,9 @@ function generate_scenario end
7171
7272
Enrich `instance_sample` with observable context drawn from `rng`.
7373
Returns a new `DataSample` extending the instance: `.x` is the final ML feature vector
74-
(possibly augmented with context features) and `.extra` holds any latent context fields
75-
needed by [`generate_scenario`](@ref).
74+
(possibly augmented with context features). Any latent fields needed by
75+
[`generate_scenario`](@ref) must go into `.context` (they are spread as kwargs via
76+
`ctx.context...`), not into `.extra`.
7677
7778
**Default**: returns `instance_sample` unchanged — no context augmentation.
7879
Non-contextual benchmarks (e.g. SVS) use this default.
@@ -111,6 +112,18 @@ Returns `(env; reset_env=true, kwargs...) -> Vector{DataSample}`, a full traject
111112
"""
112113
function generate_anticipative_solver end
113114

115+
"""
116+
objective_value(::ExogenousStochasticBenchmark, sample::DataSample, y, scenario) -> Real
117+
118+
Compute the objective value of solution `y` for a given `scenario`.
119+
Must be implemented by each concrete [`ExogenousStochasticBenchmark`](@ref).
120+
121+
This is the primary evaluation hook for stochastic benchmarks. The 2-arg fallback
122+
`objective_value(bench, sample, y)` dispatches here using the scenario stored in
123+
`sample.extra.scenario` (or averages over `sample.extra.scenarios`).
124+
"""
125+
function objective_value end
126+
114127
"""
115128
generate_parametric_anticipative_solver(::ExogenousStochasticBenchmark) -> callable
116129
@@ -216,6 +229,7 @@ function generate_dataset(
216229
rng=MersenneTwister(seed),
217230
kwargs...,
218231
)
232+
nb_instances == 0 && return DataSample[]
219233
return reduce(
220234
vcat,
221235
(
@@ -275,7 +289,7 @@ function generate_sample(
275289
generate_scenario(inner, rng; ctx.context...) for _ in 1:(saa.nb_scenarios)
276290
]
277291
if isnothing(target_policy)
278-
return [DataSample(; x=ctx.x, ctx.context..., extra=(; ctx.extra..., scenarios))]
292+
return [DataSample(; x=ctx.x, θ=ctx.θ, ctx.context..., extra=(; ctx.extra..., scenarios))]
279293
else
280294
return target_policy(ctx, scenarios)
281295
end
@@ -300,8 +314,10 @@ function generate_dataset(
300314
rng=MersenneTwister(seed),
301315
kwargs...,
302316
)
317+
nb_instances == 0 && return DataSample[]
303318
return reduce(
304-
vcat, (generate_sample(saa, rng; target_policy, kwargs...) for _ in 1:nb_instances)
319+
vcat,
320+
(generate_sample(saa, rng; target_policy, kwargs...) for _ in 1:nb_instances),
305321
)
306322
end
307323

test/dynamic_assortment.jl

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -340,10 +340,6 @@ end
340340
# Test maximizer generation
341341
maximizer = generate_maximizer(b)
342342

343-
# Test integration with sample data
344-
sample = generate_sample(b, MersenneTwister(42))
345-
@test hasfield(typeof(sample), :context)
346-
347343
environments = generate_environments(b, 3; seed=42)
348344

349345
# Evaluate policy to get data samples

0 commit comments

Comments
 (0)