QuantEcon / ContinuousDPs.jl

Continuous state dynamic programming
BSD 3-Clause "New" or "Revised" License
13 stars 10 forks source link

`set_eval_nodes!` #10

Open oyamad opened 6 years ago

oyamad commented 6 years ago

Usually we want to evaluate the optimal value, the optimal policy, and the residual at a state grid finer than the interpolation grid. The current design is that this is done by

set_eval_nodes!(res::CDPSolveResult, eval_nodes)

which modifies res.V (vector of optimal values), res.X (vector of optimal actions), and res.resid (vector of residuals).

Is this intuitive and Julian? Any alternatives?

sglyon commented 6 years ago

This does not feel intuitive to me.

I think an evaluate method that either returns a copy of res with the fields changed, a tuple of the new (V, X, resid), or some struct holding those three things makes more sense to me.

The reason for my hesitation about the current behavior is that I would expect CDPSolveResult to contain all the data used when computing the solution (cdp, tol, max_iter, ...), or generated as a byproduct of actually solving the model (V, X, resid, ...).

If we change res.V, res.X, res.resid when calling set_eval_nodes! then that no longer holds.

oyamad commented 6 years ago

That makes perfect sense. The third option (returning a struct holding V, X, resid) sounds the best to me. Do you have any idea about the name of the struct?

sglyon commented 6 years ago

I'm never usually very creative with these things and name that struct ValuePolicy in my research code.

It usually holds the value function, controls, and whatever fields I would need to evaluate those two at arbitrary points. Also having it hold resid would be natural

oyamad commented 6 years ago

ValuePolicy is very clear. I will try to find other candidate names.

I remembered I had implemented the fist option (returning (V, X, resid)) by res(s_nodes). A shortcoming of it is that one has to do, for example, _, X, _ = res(s_nodes) even when wanting only the optimal policy.

sglyon commented 6 years ago

I think putting them in a struct resolves that issue -- it is probably best to do that.

oyamad commented 6 years ago

For reference: set_eval_nodes! now accepts s_nodes_coord #14.