API rename compute/get_result to evaluate_solution/get_solution about benchopt

[mathurinm]

the way I see it, the solution is $x^* = \text{argmin} f(x)$ (exactly). Solvers only output an estimate of the solution -- all the more when run with low n_iter or high tol.

If the benchmark is not even an optimization problem, like the comparison of sparse recovery methods of @TheoGuyard, and you use a solver like OMP which does not solve an optimization problem, then it's not super what "solution" would refer too.

[tomMoral]

Yes this could be a sub-optimal solution.
But I am more in favor of result, as this is less specific to optimization than solution.

[tomMoral]

The automated call to get_result is indeed a nice improvement that could be made as the same time, as well as letting evaluate_result take an expanded dictionary as input.

[mathurinm]

@francois-rozet what bothers me with get_solution is that it's not a solution to the optimization problem. And we're heading for settings in which it's just the result of a method, not a solution.

What do you think of get_result/evaluate_result ?

[francois-rozet]

evaluate_result is good as well. However, I am not sure to understand why get_result does not return a solution. Semantically, a "solver" returns a "solution".

[agramfort]

We call it solver so it returns a solution Lets not change things for cosmetic reasons.

[mathurinm]

several people stated that it was confusing during the sprint, especially compared to get/set_dataset and get/set_objective
IMO it's not too late to change and makes onboarding easier, as #588

[tomMoral]

This is not only cosmetic, this helps with documentation and API consistency (many said during the sprint that this was a confusing point). Thus we would like to change compute to match the get_result method. The question is what it the right wording. We also want to consider the get_one_solution that is going to enter this function and the name of the object passed in the callback, to make all of this consistent.

• solution is more natural with Solver, but refers to optimization and hurts the semantic of @mathurinm 😆
• results is more general (thinking of ML benchmarks), and the nice thing is we don't change get_result.

I am in favor of this solution with: get_result/get_one_result/evaluate_result/callback(**result). This will be more consistent.
What do you think @mathurinm , @agramfort , @francois-rozet

[agramfort]

I will not fight against this. I just say that API changes are not free (deprecation, code breakage etc.) Also over the years I've seen many examples of people arguing that your API is not logic but if you change everytime someone comes with his own prior it's a pure random walk. If you want API changes just make sure it's what is best for the majority, not just the last person arguing about it.

…

Message ID: ***@***.***>

[francois-rozet]

Both solution and result are fine to me, with a preference for solution to be consistent with Solver (btw a sub-optimal solution remains a solution, same for intermediate solution). The real improvement is changing compute to evaluate_solution or evaluate_result.

P.S. if one should always call callback with result, maybe it is not necessary to force the user to give arguments to the callback. I currently write while cb(self.get_result()): in all my solvers, but while cb():should be enough. It would also allow to not count the "result creation" time as part of the run.