search

odlgroup / odl

Operator Discretization Library https://odlgroup.github.io/odl/
Mozilla Public License 2.0
371 stars 105 forks source link

Solvers API slightly inconsistent #1197

Closed kohr-h closed 6 years ago

kohr-h commented 7 years ago

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 commented 7 years ago

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

aringh commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

kohr-h commented 6 years ago

Oh yes, of course.