Creation of AbstractStochasticProgram interface #14

blegat · 2017-11-07T21:21:34Z

This big refactor improves the decoupling between the SDDP algorithm and the problem representation used by this package.

There is now 5 separate parts

The definition of an AbstractStochasticProgram <: LightGraphs.AbstractGraph, this aims to get as close as possible as @odow 's definition. Currently it is lacking Noise (triangle node). Noise cannot always be represented by a graph since the number of possible value of the noise might be infinite (e.g. Normally distributed). In this case, we may rather view the node model as being parametrized by a continuous parameter instead of several different nodes. @frapac @odow Do you prefer AbstractStochasticProgram or AbstractStochasticProblem ? This may be replaced by a separate StochasticProblem.jl package containing @odow 's definition in Julia.
SDDP specific abstract components : stats.jl, stopcrit.jl, cutgen.jl, sampler.jl, solution.jl. Some of these parts may be specific to the SDDP algorithm but some might be replaced by a definition in StochasticProblem.jl.
SDDP implementation only using the abstraction above : path.jl, sddp.jl.
StochasticProgram : Generic implementation of AbstractStochasticProgram using LightGraphs' DiGraph and MathProgBase to solve the subprograms.
Transformation from StructJuMP model into StochasticProgram.

codecov · 2017-11-07T21:21:36Z

Codecov Report

Merging #14 into master will increase coverage by 0.21%.
The diff coverage is 91.66%.

@@            Coverage Diff             @@
##           master      #14      +/-   ##
==========================================
+ Coverage   87.93%   88.14%   +0.21%     
==========================================
  Files          14       14              
  Lines         870      869       -1     
==========================================
+ Hits          765      766       +1     
+ Misses        105      103       -2

Impacted Files	Coverage Δ
src/cutstore.jl	`72.72% <100%> (ø)`	⬆️
src/waitandsee.jl	`91.07% <100%> (+0.16%)`	⬆️
src/stochprog.jl	`100% <100%> (ø)`
src/interface.jl	`100% <100%> (ø)`	⬆️
src/sampler.jl	`89.13% <85.71%> (+0.24%)`	⬆️
src/nlds.jl	`87.53% <87.5%> (-0.87%)`	⬇️
src/graph.jl	`90.76% <90.76%> (-0.14%)`	⬇️
src/sddp.jl	`83.87% <90.9%> (+0.53%)`	⬆️
src/cutgen.jl	`89.28% <92.85%> (+1.05%)`	⬆️
src/path.jl	`96.96% <93.75%> (-0.05%)`	⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bece242...9931b19. Read the comment docs.

odow · 2017-11-07T21:24:29Z

I'm going to need some time to go through this and understand the internals of your implementation a bit better.

odow · 2017-11-07T21:27:36Z

src/solution.jl

@@ -0,0 +1,33 @@
+# π, σ and ρ do not really make sense alone so only
+# their product will T, h, d, e is stored
+mutable struct Solution


Can you expand a bit on these definitions? Provide a doc-string?

I have hidden this into the nlds.jl file and replaced it with an AbstractSolution interface

odow

I'm a fan of the direction in stochprog.jl. I think a MOI for SDDP is good

I see a basic form as:

A SDDPModel is a graph with nodes.
Each node is a subproblem that has a collection of stagewise independent noises, takes an incoming state, and returns an objective, and dual vector for the state

For a node you want to be able to

set the state
query the outgoing state of a solved node
loop through the noises
set a particular noise
solve
get the objective and dual vector
add a cut given a list of objectives, dual vectors, probabilities, and an exiting state (and a risk measure)
run cut selection

For the graph you want to be able to

identify the root node
find the children of a node
find the probability of the exiting arcs
walk the graph

odow · 2017-11-07T21:29:05Z

src/stochprog.jl

+
+Creates a stochastic program from the arguments
+"""
+function stochasticprogram end


StochasticProgram? CamelCase?

I am open to using CamelCase. I am used to using lowercase in such cases since it is a function and not a type.

odow · 2017-11-07T21:31:01Z

src/stochprog.jl

+
+Sets the parent solution of `child` as `sol`, the solution obtained at `node`.
+"""
+function setchildx! end


I'm not sure what this means? Should it be setchildstate!(sp, parent, child)?

Yes the name is awful and the argument list is even worse :D
For some obscure reason I need to also give the parent as parameter (because of the childT I used for the EntropicCone to account for the permutation tranformations).
I prefer setchildstate!(sp, child, sol) because in setchildstate!(sp, parent, child), we need to get the outgoing state of parent but in the mean time it might have been solved by another thread and the solution might not be the one we want.

Then it should just be setincomingstate!(sp, child, state::Vector{T})?

In case the parent is unbounded, the solution also contain an unbounded ray (which is composed of a feasible point and an unbounded vector). In this case, passing just a vector is not enough. Passing an AbstractSolution provides enough flexibility to handle this case.

odow · 2017-11-07T21:31:42Z

src/stochprog.jl

+
+Gets the current lower bound to the objective of `node`.
+"""
+function getobjlb end


I think we should aim for readability over conciseness getobjectivebound? Because it might be maximization

I agree, that's better :)

Done iin 9dafe36

odow · 2017-11-07T21:31:53Z

src/stochprog.jl

+
+Sets the lower bounds to the objective of the children of `node` to `θlb`.
+"""
+function setθlb! end


Ditto. see above

Done in 9dafe36

odow · 2017-11-07T21:32:20Z

src/stochprog.jl

+
+Returns the dimension of the state at `node`.
+"""
+function statedim end


I prefer statedim(sp). The state dimension should not change at different nodes in the graph

In the EntropicCone application, it does :/

What is a "state"? If we think about reservoirs, there might be 2 in the first stage, and 3 in the second. However, there really is 3 in every stage, it's just that in the first stage the 3rd reservoir has a volume of 0. State dimension is a constant in a stochastic program.

When it has a physical meaning, I agree but in an abstract mathematical stochastic problem (it is the case with the entropic cone), there is not clear relation between the states from one node to the other one and the number change. In fact with the entropic cone, the graph is infinite and deep down in the graph, there are nodes of infinite number of states !

This is because you're only using feasibility cuts right? I still don't fully understand your model haha

I don't think it is related to feasibility cuts

odow · 2017-11-07T21:32:48Z

src/stochprog.jl

+
+Returns the id of `edge` for `src(edge)`. This id should be between 1 and `outdegree(src(edge))`. In case of multiple cuts, the value `θ[edgeid(sp, edge)]` at `src(edge)` corresponds to the lower bound to the objective value of `dst(edge)`.
+"""
+function edgeid end


What is this necessary?

It has been now removed :)

odow · 2017-11-07T21:33:09Z

src/stochprog.jl

+
+Sets the probability to take the edge `edge` in the stochastic problem `sp`.
+"""
+function setprobability! end


Ditto about conciseness s/proba/probability

Done in da010de

coveralls · 2017-11-07T22:29:28Z

Coverage increased (+0.08%) to 88.014% when pulling c5462ba on stochprog into 9cfc4a8 on master.

coveralls · 2017-11-07T22:29:28Z

Coverage increased (+0.08%) to 88.014% when pulling c5462ba on stochprog into 9cfc4a8 on master.

coveralls · 2017-11-07T23:48:11Z

Coverage increased (+0.1%) to 88.042% when pulling fc99a5a on stochprog into 9cfc4a8 on master.

coveralls · 2017-11-08T01:26:23Z

Coverage increased (+0.02%) to 87.953% when pulling 9dafe36 on stochprog into 9cfc4a8 on master.

coveralls · 2017-11-08T10:06:05Z

Coverage increased (+0.2%) to 88.147% when pulling 8dff241 on stochprog into 9cfc4a8 on master.

coveralls · 2017-11-08T10:06:06Z

Coverage increased (+0.2%) to 88.147% when pulling 8dff241 on stochprog into 9cfc4a8 on master.

coveralls · 2017-11-08T11:12:21Z

Coverage increased (+0.2%) to 88.147% when pulling 8dff241 on stochprog into 9cfc4a8 on master.

coveralls · 2017-11-08T11:56:52Z

Coverage increased (+0.2%) to 88.147% when pulling c9b60d1 on stochprog into 9cfc4a8 on master.

frapac

Good job! I think this work bring closer a common implementation of SDDP

I have some general comments :

concerning continuous probability law, I agree that this remains an issue. A solution would be to quantize continuous probability laws (however, I do not know any Julia package that implements the methods of Pagès et al.)
a main issue is the way we model uncertainty : in this implementation we affect a proba for each edge outgoing a given node, if I understood well. I do not know if it is well suited if we have stagewise independency, that is, we have only a single node per time steps. Maybe we should implement uncertainty inside the node rather than in the edges, and point to the following node directly inside the definition of the uncertainty model. What do you think? The point is that we should be able to model easily both lattice structures and Markov chain model. I have to admit that this point is unclear to me ...

I think that the power of a generic implementation is that it is not specific to SDDP. We may also want to implement a proper Dynamic Programming solver, or other methods (such as the particle methods described by Dallagi, or spatial decomposition algorithms).
Concerning @odow comment, I think we should also be able to define properly measurability constraint inside a given node (e.g Here and Now and Wait and See controls, etc), and be able to fetch easily the Bellman value function affected to this particular node. The SOLIB draft was a good starting point to figure out what elements do we need to model a node in a stochastic program!

If @odow is ok, I would be eager to create a new JuliaStochOpt organization on Github :)

frapac · 2017-11-08T12:16:27Z

src/waitandsee.jl

@@ -1,7 +1,7 @@
 export waitandsee

-mutable struct WSPath
-    node::SDDPNode
+mutable struct WSPath{NodeT}


Wouldn't it be clearer to write WaitAndSeePath? I tend to think that explicit is always better than implicit^^

frapac · 2017-11-08T12:18:46Z

src/stochprog.jl

+
+Sets the bounds to the objective of the child `child` of `node` to `θlb`.
+"""
+function setθbound! end


Maybe we should explicit theta as BellmanFunction, but I am not sure. What do you think?

Yes, bellman function seems better.

odow · 2017-11-08T19:49:22Z

a main issue is the way we model uncertainty : in this implementation we affect a proba for each edge outgoing a given node, if I understood well. I do not know if it is well suited if we have stagewise independency, that is, we have only a single node per time steps

Stagewise indepedent noises live inside a node. Nodes are connected by edges which have proabilities associated with them. The edge and noise inside a node are sampled independently.

odow · 2017-11-08T19:50:39Z

I think that the power of a generic implementation is that it is not specific to SDDP. We may also want to implement a proper Dynamic Programming solver, or other methods (such as the particle methods described by Dallagi, or spatial decomposition algorithms).

+100

coveralls · 2017-11-09T14:36:53Z

Coverage increased (+0.2%) to 88.147% when pulling 2f2d3f2 on stochprog into 9cfc4a8 on master.

coveralls · 2017-11-09T14:36:53Z

Coverage increased (+0.2%) to 88.147% when pulling 2f2d3f2 on stochprog into 9cfc4a8 on master.

blegat · 2017-11-09T19:42:15Z

a main issue is the way we model uncertainty : in this implementation we affect a proba for each edge outgoing a given node, if I understood well. I do not know if it is well suited if we have stagewise independency, that is, we have only a single node per time steps. Maybe we should implement uncertainty inside the node rather than in the edges, and point to the following node directly inside the definition of the uncertainty model. What do you think?

The interface tries to be agnostic on the particular representation of the stochastic problem.
The probability(sp, edge) method has the advantage that it still makes sense for the markovian case.
If you have stagewise independence, you will clearly not store internally a Dict{Edge, Float64} but rather do what you do in StochDynamicProgramming because that would be a lot more efficient.
From the point of view of the SDDP algorithm the way the lattice/graph is represented does not really matter and probability(sp, edge) is as convenient as it could be right ?
What advantage would we gain to constraint the interface to be less agnostic of the representation and closer to a simple stagewise independent structure ?

blegat · 2017-11-13T14:43:55Z

@frapac thinking a bit more about it, there is indeed an important issue here with cut sharing.
If you generate an averaged optimality cut, you can share it with all the nodes that have the same children with same probabilities than you. We could use the noise node of @odow 's framework to model this. Even if the case of a graph, if two node share the same noise node, they will share the same cuts.
In the algorithm, when we generate an averaged cut, instead of sending it to a specific node, we can send it to the noise node.

odow · 2017-11-13T19:07:00Z

I've moved away from explicit noise nodes to a hazard-decision and a decision-hazard thing https://www.sharelatex.com/project/59556d23c8f18fc31bc955d5

frapac · 2017-11-13T20:58:36Z

@blegat I agree with this particular issue. I think that your solution, combined with @odow 's combination of nodes and edges nodes, would solve this issue. But I hope that this implementation would not add a new layer of complexity ...

odow · 2017-11-13T21:07:44Z

If you generate an averaged optimality cut, you can share it with all the nodes that have the same children with same probabilities than you.

Better to solve all the children, then, if another node has the same children, you can recompute the averaged cut (using possibly different probabilities) without resolving.

blegat · 2017-11-13T23:42:51Z

Better to solve all the children, then, if another node has the same children, you can recompute the averaged cut (using possibly different probabilities) without resolving.

What would the interface be ? Something like the following ?

the SDDP algorithm informs the Stochastic Program that it has solved a set of nodes and gives the corresponding cuts.
Internally the Stochastic Program, hash the set of children to determines the nodes for which it can computes the averaged cut and gives them new cuts (or do something faster in case of more particular structure like stagewise independent without any graph structure).

odow · 2017-11-14T00:06:18Z

Different backward pass sampling schemes would do it differently.

One approach is to recurse back up the tree. After adding a cut to a node, check the other children of its parent to see if it is possible to add a cut. That will work best for lattice structures.

There are probably a number of optimizations and different heuristics you can use for deciding which nodes (and when) to add a cut.

coveralls · 2017-11-14T00:31:51Z

Coverage increased (+0.2%) to 88.147% when pulling 9931b19 on stochprog into bece242 on master.

blegat · 2017-11-14T10:02:43Z

I see, so you mean the interface should remain more flexible so that the SDDP algorithm can make the choice.

I will make this huge PR now, it does not mean that I consider the discussion closed but rather that we should transition the discussion to a StochasticProgram.jl package :)

odow reviewed Nov 7, 2017

View reviewed changes

frapac reviewed Nov 8, 2017

View reviewed changes

blegat force-pushed the stochprog branch from 2f2d3f2 to 9931b19 Compare November 13, 2017 23:43

blegat added 6 commits November 14, 2017 00:44

Creation of AbstractStochasticProgram interface

85eaed6

Improve solution abstraction

067aa16

Improve objective bounds interface

c78f909

Improves solution interface and get rid of edgeid

ab5457d

proba -> probability

cf2d948

Reenable solvers

3d19a6c

blegat added 2 commits November 14, 2017 00:44

Fix 32 bits

e8f07f2

WS -> WaitAndSee

9931b19

blegat merged commit 1d0cbe6 into master Nov 14, 2017

Creation of AbstractStochasticProgram interface #14

Creation of AbstractStochasticProgram interface #14

Conversation

blegat commented Nov 7, 2017

codecov bot commented Nov 7, 2017 • edited Loading

Codecov Report

odow commented Nov 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

odow left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Nov 7, 2017

coveralls commented Nov 7, 2017

coveralls commented Nov 7, 2017

coveralls commented Nov 8, 2017 • edited Loading

coveralls commented Nov 8, 2017

coveralls commented Nov 8, 2017

coveralls commented Nov 8, 2017 • edited Loading

coveralls commented Nov 8, 2017 • edited Loading

frapac left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

odow commented Nov 8, 2017

odow commented Nov 8, 2017

coveralls commented Nov 9, 2017

coveralls commented Nov 9, 2017

blegat commented Nov 9, 2017

blegat commented Nov 13, 2017

odow commented Nov 13, 2017

frapac commented Nov 13, 2017

odow commented Nov 13, 2017

blegat commented Nov 13, 2017

odow commented Nov 14, 2017

coveralls commented Nov 14, 2017 • edited Loading

blegat commented Nov 14, 2017

codecov bot commented Nov 7, 2017 •

edited

Loading

odow left a comment •

edited

Loading

coveralls commented Nov 8, 2017 •

edited

Loading

coveralls commented Nov 8, 2017 •

edited

Loading

coveralls commented Nov 8, 2017 •

edited

Loading

coveralls commented Nov 14, 2017 •

edited

Loading