Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistency #701

Closed
pauleseifert opened this issue Oct 23, 2023 · 3 comments · Fixed by #706
Closed

Inconsistency #701

pauleseifert opened this issue Oct 23, 2023 · 3 comments · Fixed by #706

Comments

@pauleseifert
Copy link

Hi Oscar,

I found a small inconsistency in the documentation of the stopping rule. According to the documentation, passing a vector of stopping criteria results in an AND logic for all passed conditions. Reading from the documentation:

StoppingChain(rules::AbstractStoppingRule...)
Terminate once all of the rules are statified.
This stopping rule short-circuits, so subsequent rules are only tested if the previous pass.

However, passing SDDP.IterationLimit() in the vector overwrites the iteration limit in the SDDP.train() function, if empty, and thus results in an OR logic and stops once the iteration limit is reached.

```     if time_limit !== nothing
        push!(stopping_rules, TimeLimit(time_limit))
    end
@odow
Copy link
Owner

odow commented Oct 23, 2023

So the stopping_rules argument to train

SDDP.jl/src/algorithm.jl

Lines 873 to 874 in cb89e07

- `stoping_rules`: a vector of [`SDDP.AbstractStoppingRule`](@ref)s. Defaults
to [`SimulationStoppingRule`](@ref).

accepts a vector of rules. Time and iterations limits are added if specified:

SDDP.jl/src/algorithm.jl

Lines 1057 to 1072 in cb89e07

# Convert the vector to an AbstractStoppingRule. Otherwise if the user gives
# something like stopping_rules = [SDDP.IterationLimit(100)], the vector
# will be concretely typed and we can't add a TimeLimit.
stopping_rules = convert(Vector{AbstractStoppingRule}, stopping_rules)
# Add the limits as stopping rules. An IterationLimit or TimeLimit may
# already exist in stopping_rules, but that doesn't matter.
if iteration_limit !== nothing
push!(stopping_rules, IterationLimit(iteration_limit))
end
if time_limit !== nothing
push!(stopping_rules, TimeLimit(time_limit))
end
# If no stopping rule exists, add the default rule.
if isempty(stopping_rules)
push!(stopping_rules, SimulationStoppingRule())
end

The elements in stopping_rules act as an OR: we will stop if any of the rules are true.

StoppingChain is a particular stopping rule that uses an AND criteria:

"""
StoppingChain(rules::AbstractStoppingRule...)
Terminate once all of the `rules` are statified.
This stopping rule short-circuits, so subsequent rules are only tested if the
previous pass.
## Examples
A stopping rule that runs 100 iterations, then checks for the bound stalling:
```julia
StoppingChain(IterationLimit(100), BoundStalling(5, 0.1))
```
"""

You can pass a StoppingChain as one of the rules to stopping_rules.

SDDP.train(model; stopping_rules = [SDDP.StoppingChain(SDDP.IterationLimit(100), SDDP.TimeLimit(100.0))])

Can you like to part of the docs where this could be clarified?

@pauleseifert
Copy link
Author

Right. This applies only if using the stopping chain. I was expecting the same behaviour by only using a vector of stopping rules to automatically convert to a stopping chain. Then a small addition to the documentation in

would do the trick for me.

Stopping rules

In addition to the limits provided as keyword arguments, a variety of other
stopping rules are available. These can be passed to SDDP.train
as a vector to the stopping_rules keyword. Training stops if any of these rules becomes active. For a union of stopping rules use StoppingChain().

For_ example:

SDDP.train(model, stopping_rules = [SDDP.BoundStalling(10, 1e-4), SDDP.TimeLimit(100.0)])

SDDP.train(model; stopping_rules = [SDDP.StoppingChain(SDDP.IterationLimit(100), SDDP.TimeLimit(100.0))])

See [Stopping rules](@ref api_stopping_rules) for a list of stopping rules
supported by SDDP.jl.

@odow
Copy link
Owner

odow commented Oct 28, 2023

Here you go: #706

@odow odow closed this as completed in #706 Oct 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants