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

Solvers API slightly inconsistent #1197

Closed
kohr-h opened this issue Oct 18, 2017 · 9 comments
Closed

Solvers API slightly inconsistent #1197

kohr-h opened this issue Oct 18, 2017 · 9 comments

Comments

@kohr-h
Copy link
Member

kohr-h commented Oct 18, 2017

Of the convex solvers we have so far, most have the structure that f is not composed, and the compositions with operators happen in the g functional(s). Only PDHG does it the other way around.

Should we change it?

I'm asking since I'm gonna add linearized ADMM which solves the same problems as PDHG, and I want this to be settled before.

@kohr-h
Copy link
Member Author

kohr-h commented Oct 18, 2017

Heads-up to @mehrhardt and @adler-j for comments.

@kohr-h kohr-h mentioned this issue Oct 18, 2017
@aringh
Copy link
Member

aringh commented Oct 19, 2017

This falls down to what I always ask: consistency with the paper or not? :-p As long as the documentation is clear I don't think it really matters.

@adler-j
Copy link
Member

adler-j commented Oct 19, 2017

My suggestion is either rename to primal_functional, which would solve the naming, or leave names as is and simply keep the argument order the same.

@mehrhardt
Copy link
Contributor

I think it would be good if our algorithms are consistent. That is more important than the consistency with the papers. To me, it is more natural that f is composed with an operator as the data term often comes first and f < g in a lexiographic sense. I suppose some of you feel the other way :)

@kohr-h
Copy link
Member Author

kohr-h commented Oct 19, 2017

My suggestion is either rename to primal_functional, which would solve the naming, or leave names as is and simply keep the argument order the same.

Not so sure about that. For some algorithms that are not really of primal-dual type (like ADMM?) the names "primal" and "dual" would be quite a stretch.

I think it would be good if our algorithms are consistent. That is more important than the consistency with the papers.

I fully agree with this. In papers, authors use whatever they like best, and if we mirror every paper notation we end up with a hopeless mess.

There's also a practical side to it: when you want to switch between two solvers that handle exactly the same type of problem (like PDHG and ADMM), you shouldn't have to switch the roles of the functionals every time. The same holds when using Forward-Backward or Douglas-Rachford with just one g functional.

To me, it is more natural that f is composed with an operator as the data term often comes first and f < g in a lexiographic sense. I suppose some of you feel the other way :)

This point is perhaps more a matter of taste and practicality. Going with f composed and g not would mean that we need to change everything except PDHG, the other possibility means only changing PDHG.

@adler-j
Copy link
Member

adler-j commented Feb 19, 2018

Did we arrive at a decision here, I'm currently doing this at @kohr-h's suggestion, but I'm not quite sure what to do. For sure, only changing pdhg is the easier route.

@mehrhardt
Copy link
Contributor

I think both here and at other places, it would be good to make a decision such that we can go forward. Given that every solver has their own range of problems that it is applicable to, this "decision" can and should be handled in a flexible manner. However, for the most standard problem class it seems that we now settled on f(x) + g(Ax) so that only PDHG needs to be changed.

@aringh
Copy link
Member

aringh commented Jun 19, 2018

Can this be considered done, given #1286 where the interface of PDHG was changed?

@kohr-h
Copy link
Member Author

kohr-h commented Jun 19, 2018

Oh yes, of course.

@kohr-h kohr-h closed this as completed Jun 19, 2018
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

4 participants