Consistent naming convention for solvers

[jasonmcewen]

Since we've added and are adding a number of new solvers, we probably need to develop a consistent naming approach. For example, we currently have sopt_l1_solver (Douglas-Rachford), sopt_l1_sdmm (SDMM) and sopt_l1_solver_aadmm (approximate ADMM), and also reweighted versions. In future, we might also like to support the unconstrained, as well as the constrained problem (i.e. + lambda * data fidelity term).

Renaming will be a bit of a pain, since it would be a breaking change, but it's probably best to sort out now rather than later (we can also add in old interfaces that could be depreciated in time).

How about sopt_l1solver? For example, sopt_l1_solver_constrained_aadmm_rw or sopt_l1_solver_unconstrained_sdmm. Or is that convention too heavy?

@rafael-carrillo @mdavezac @vijaykartik Thoughts? Suggestions?

[rafael-carrillo]

Hi all,

Why don’t we drop the “solver” part of the name and just have something like “sopt_l1_dr” or “sopt_l1_padmm”.

One more think: I would prefer to name the admm solver proximal dam (padmm) instead of approximate admm. Approximate is quite vague since includes many options. Proximal is a bit more accurate and it’s also the way it’s referred to inn the community.

Let me know what you think.

On 23 Mar 2015, at 15:15, Jason McEwen notifications@github.com wrote:

Since we've added and are adding a number of new solvers, we probably need to develop a consistent naming approach. For example, we currently have sopt_l1_solver (Douglas-Rachford), sopt_l1_sdmm (SDMM) and sopt_l1_solver_aadmm (approximate ADMM), and also reweighted versions. In future, we might also like to support the unconstrained, as well as the constrained problem (i.e. + lambda * data fidelity term).

Renaming will be a bit of a pain, since it would be a breaking change, but it's probably best to sort out now rather than later (we can also add in old interfaces that could be depreciated in time).

How about sopt_l1solver? For example, sopt_l1_solver_constrained_aadmm_rw or sopt_l1_solver_unconstrained_sdmm. Or is that convention too heavy?

@rafael-carrillo https://github.com/rafael-carrillo @mdavezac https://github.com/mdavezac @vijaykartik https://github.com/vijaykartik Thoughts? Suggestions?

— Reply to this email directly or view it on GitHub https://github.com/astro-informatics/sopt/issues/6.

[jasonmcewen]

Algo names sound good! So, dr, sdmm, padmm.

I do quite like having "solver" since it is very descriptive for general users (which hopefully we will have with the new project and planned extensions!) but I'm not completely fixed on the solver part. Maybe l1solver, rather than ..._l1solver... to keep things (slightly) most concise?

And what about the re-weighting? Shall we have a rw suffix to specify that?

And the constrained vs unconstrained?

[jasonmcewen]

Guys, what is the verdict on this? Would be good to make the changes soon.

[vijaykartik]

My suggestion:

soptsolver_l1_dr/sdmm/padmm and so on.

This way we kind of go one level down at each underscore. Users might find it easier to remember a "menu"-style name, so: solver? yes, then: l1? (or TV, for e.g.) yes? then: how? sdmm/padmm etc.

The way I am using reweighting at the moment is basically to always use the reweighted solver and set it to zero reweightings if i want to run through just once. So instead of having a suffix we could keep it as a parameter. I don't know if this makes sense or if it's completely stupid though.

[jasonmcewen]

Yep, I like the idea of going one level down with each underscore, which is the convention we've essentially been trying to follow. However, I think we should start with sopt_ at the top level since that is the package name and is used throughout. Then all sopt functions begin sopt_*. Starting from the highest level and working down is a great idea.

The only problem with always using the reweighted version is that users need to set all of the corresponding parameters. I would therefore vote for exposing interfaces both with and without reweighting.

So how about sopt_solver_l1_problem_algo_weighting, where:

problem = {constrained, unconstrained} or {constr, unconstr} algo = {dr, sdmm, padmm} weighting = { reweight, empty if don't RW} or {rw, empty if don't reweight}

Just to ensure this is clear, examples would include: sopt_solver_l1_unconstr_padmm, sopt_solver_l1_constr_sdmm_rw

[rafael-carrillo]

I like this idea Jason. For the reweighted version maybe we just keep rw and empty if there is no reweighting.

BTW I have a Matlab code for the unconstrained problem that uses FISTA acceleration and the same L1 prox as DR or ADMM. I will push it to the repo. I might even have a first C version in my archives.

See you later.

Cheers,

Rafa

Sent from my iPhone

On Mar 25, 2015, at 2:46 AM, Jason McEwen notifications@github.com wrote:

Yep, I like the idea of going one level down with each underscore, which is the convention we've essentially been trying to follow. However, I think we should start with sopt at the top level since that is the package name and is used throughout. Then all sopt functions begin sopt*. Starting from the highest level and working down is a great idea.

The only problem with always using the reweighted version is that users need to set all of the corresponding parameters. I would therefore vote for exposing interfaces both with and without reweighting.

So how about sopt_solverl1, where:

= {constrained, unconstrained} or {constr, unconstr} = {dr, sdmm, padmm} = { reweight, empty if don't RW} or {rw, empty if don't reweight}

Just to ensure this is clear, examples would include: sopt_solver_l1_unconstr_padmm, sopt_solver_l1_constr_sdmm_rw

— Reply to this email directly or view it on GitHub.

[jasonmcewen]

Cool! Would be great to also support the unconstrained problem at some point and if you have any implementations already that would be excellent!

@vijaykartik Any further comments on the naming conventions?

These naming conventions should also percolate to the parameter structs in sopt_l1.h as well.

Any takers to make the updates? We should do so on a fresh branch I think, which should perhaps be a branch of real-data(?), since that is where a lot of development is ongoing at present. It would also be useful to have some help from @mdavezac for updating the python bindings.

[vijaykartik]

Function and struct names will a bit long now, but that shouldn't be a dealbreaker I suppose. So, we're going with sopt_solver_l1_problem_algo[_rw] ? I can set up a new branch and refactor the source if we freeze the naming here.

[rafael-carrillo]

Yes! Though I still don’t like the “solver” part. SOPT is primary concerned with the solution of convex optimisation problems. We have both l1 and tv problems (I think l2 as well) and we solve them. If we incorporate unconstrained problems then we need to specify it. In the future if we include joint sparsity problems like nuclear norm minimisation, this would be also a solver.

On 25 Mar 2015, at 10:19, vijaykartik notifications@github.com wrote:

Function and struct names will a bit long now, but that shouldn't be a dealbreaker I suppose. So, we're going with sopt_solver_l1_problem_algo[_rw] ? I can set up a new branch and refactor the source if we freeze the naming here.

— Reply to this email directly or view it on GitHub https://github.com/astro-informatics/sopt/issues/6#issuecomment-85948694.

[jasonmcewen]

Thanks for the comments! Ok, I take your point about "solver". However, for a user who isn't at all familiar with the code I think they might look for "solver" functions. We would indeed need to update the l2 and tv functions too.

I think we need further opinions. @mdavezac, what do you think?

[mdavezac]

@jasonmcewen, I'm guessing this is now somewhat out of scope? Close?

In c++, everything should be in the sopt or purify namespace. The algorithms per se are in sopt::algorithm.

[jasonmcewen]

Yes, this was all related to the old C version of the code. Now that is superseded by the C++ version this issue it indeed out of scope.