Refactor tests: R2N

[MaxenceGollier]

@dpo @MohamedLaghdafHABIBOULLAH, related to #130.

This is very much WIP, but you can already take a look.
I did a proof of concept with R2N, all solvers will eventually have similar tests.

• There should be one function that does most of the test, this function is test-solver.jl
• each solver should have its own test file
• allocations test don't need to be separated from the other tests
• Just testing if the algorithm completes on the bpdn problem is not sufficient
• having verbose > 0 adds too much information/noise into the test results

[MaxenceGollier]

LSR1 is allocating, will try to fix in LinearOperators.jl

[MohamedLaghdafHABIBOULLAH]

Thank you @MaxenceGollier very thoughtful, having the algorithms tested on different problems (toys, bpdn and others) will be very useful!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.74%. Comparing base (e0f214d) to head (8dcdd0b).
⚠️ Report is 259 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master     #271       +/-   ##
===========================================
+ Coverage   61.53%   84.74%   +23.21%     
===========================================
  Files          11       13        +2     
  Lines        1292     1626      +334     
===========================================
+ Hits          795     1378      +583     
+ Misses        497      248      -249     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[MaxenceGollier]

@dpo, @MohamedLaghdafHABIBOULLAH,

maybe we merge this first and then do the other solvers ? Or do I just do everything at once here ?
Let me know.

[MohamedLaghdafHABIBOULLAH]

@MaxenceGollier I think you should keep this PR tested only with R2N, so that it is easy to review and we will add the other solvers in different PR!

[MohamedLaghdafHABIBOULLAH]

could you open an issue for this comment?

[MaxenceGollier]

I think it is just a bad idea to add the allocations tests inside this function because this means that you can not assign a variable to one of the solver_constructor_kwargs or solver_kwargs, or else they will get boxed and result in allocations.

To be clearer @wrappedallocs works in such a way that even if

@wrappedallocs solve!(solver, reg_nlp, stats; atol = 1e-3)

returns 0,

tol = 1e-3
@wrappedallocs solve!(solver, reg_nlp, stats; atol = tol)

doesn't.

I think adding this constraint is very confusing so we should just leave the allocation tests out of this function.
I want to keep the comment though so that users will understand why we did not add the allocation tests there, but it might be more precise.

[MohamedLaghdafHABIBOULLAH]

Can't we have a function wrappedallocsv2 to avoid this?

[MohamedLaghdafHABIBOULLAH]

@MaxenceGollier
I checked this in detail. The issue is actually related to R2DH, and we should probably open a dedicated issue. The problem occurs when $$s = 0$$. In this case, R2DH does not stop because $$\xi = h_k - m_k(s) + \max(1, |h_k|)\times 10 \times \varepsilon = \max(1, |h_k|)\times 10 \times \varepsilon.$$
Although this quantity may be small, it is not negligible. When we compute its root, the resulting value can be larger than expected, which may prevent the algorithm from terminating.

A simple and reasonable fix is to assume instead that $$\xi = h_k - m_k(s).$$

[MaxenceGollier]

I never understood why we added this term to $$\xi$$ @dpo ?