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

set_eval_nodes! #10

Open
oyamad opened this issue Nov 20, 2017 · 6 comments
Open

set_eval_nodes! #10

oyamad opened this issue Nov 20, 2017 · 6 comments

Comments

@oyamad
Copy link
Member

oyamad commented Nov 20, 2017

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
Copy link
Member

sglyon commented Nov 20, 2017

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
Copy link
Member Author

oyamad commented Nov 20, 2017

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
Copy link
Member

sglyon commented Nov 20, 2017

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
Copy link
Member Author

oyamad commented Nov 20, 2017

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
Copy link
Member

sglyon commented Nov 20, 2017

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

@oyamad
Copy link
Member Author

oyamad commented Nov 21, 2017

For reference: set_eval_nodes! now accepts s_nodes_coord #14.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants