Dev sdp search spaces #143
Conversation
|
Good job! Could you benchmark DP in master and in this branch, just to see by how much DP was improved? |
src/objects.jl
Outdated
| @@ -113,6 +113,8 @@ type StochDynProgModel <: SPModel | |||
| constraints::Function | |||
| noises::Vector{NoiseLaw} | |||
|
|
|||
| build_search_space::Union{Void, Function} | |||
There was a problem hiding this comment.
Instead of Union, the Julia's doc advices to use the Nullable type (as far as I understand, it is better for the compiler)
https://docs.julialang.org/en/release-0.4/manual/types/#nullable-types-representing-missing-values
src/sdpLoops.jl
Outdated
| the indexes of the variable | ||
|
|
||
| """ | ||
| function index_from_variable(variable, bounds::Array, variable_steps::Array) |
There was a problem hiding this comment.
What do you think of this signature instead
function index_from_variable{T}(variable::Vector{T}, bounds::Vector{T}, variable_steps::Vector{T})
src/sdpLoops.jl
Outdated
| the indexes of the variable | ||
|
|
||
| """ | ||
| function is_next_state_feasible(next_state, x_dim, x_bounds) |
There was a problem hiding this comment.
Maybe we should add the types in the function's signature?
src/sdpLoops.jl
Outdated
| function sdp_dh_get_u(sampling_size, samples, probas, u_bounds, | ||
| x_bounds, x_steps, x_dim, product_controls, | ||
| dynamics, constraints, cost, Vitp, t, x, w, | ||
| build_Ux = nothing) |
There was a problem hiding this comment.
What if we factorize the arguments of this function in a dedicated type (as SDPLoop or whatever)?
I think that would DRY a bit the code, and makes the signature of this function and the following ones nicer.
Codecov Report
@@ Coverage Diff @@
## release-0.4 #143 +/- ##
==============================================
Coverage ? 87.87%
==============================================
Files ? 12
Lines ? 767
Branches ? 0
==============================================
Hits ? 674
Misses ? 93
Partials ? 0
Continue to review full report at Codecov.
|
src/sdp.jl
Outdated
| @@ -0,0 +1,460 @@ | |||
| # Copyright 2015, Vincent Leclere, Francois Pacaud, Henri Gerard and | |||
There was a problem hiding this comment.
Replace name by first letter and dot, like V. Leclere
src/sdpLoops.jl
Outdated
| discretization step for each component | ||
|
|
||
| # Returns | ||
| * `index::Tuple{Integeres}`: |
This branch introduces the possibility to compute a complex control search space knowing t,x and eventually w. Forward simulations are parallelized. Naming is more consistent with SDDP. SDP implementation is more modular and IMO cleaner. The next step is to delete StochDynProgModel to be fully consistent with SDDP. I will take care of issue #138 at the same time.