Refactor tests: R2N by MaxenceGollier · Pull Request #271 · JuliaSmoothOptimizers/RegularizedOptimization.jl

MaxenceGollier · 2025-12-18T16:28:00Z

@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 · 2025-12-18T16:58:03Z

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

MohamedLaghdafHABIBOULLAH · 2025-12-23T00:36:23Z

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

codecov · 2026-01-03T09:30:12Z

Codecov Report

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

Additional details and impacted files

@@             Coverage Diff             @@
##           master     #271       +/-   ##
===========================================
+ Coverage   61.53%   84.72%   +23.19%     
===========================================
  Files          11       13        +2     
  Lines        1292     1624      +332     
===========================================
+ Hits          795     1376      +581     
+ 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 · 2026-01-03T10:41:15Z

@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 · 2026-01-04T03:52:49Z

@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 · 2026-01-04T05:43:17Z

test/test-solver.jl

+  # Remove the x0 entry from solver_kwargs
+  optimized_solver_kwargs = Base.structdiff(solver_kwargs, NamedTuple{(:x0,)})
+  solve!(solver, reg_nlp, stats_optimized; x = x0, optimized_solver_kwargs...) # It would be interesting to check for allocations here as well but depending on 
+  # the structure of solver_kwargs, some variables might get boxed, resulting in 


could you open an issue for this comment?

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.

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

No, there is nothing "easy" we can do, keyword arguments will get boxed, this is a Julia issue.

MohamedLaghdafHABIBOULLAH · 2026-01-04T05:57:28Z

test/test-R2N.jl

+      solve!(solver, reg_nlp, stats, σk = 1.0, β = 1e16, atol = 1e-6, rtol = 1e-6)
+    ) == 0
+
+    #test_solver(reg_nlp,  # FIXME: divide by 0 error in the LBFGS approximation


@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).$$

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

That term is there to compensate for catastrophic cancellation between $h_k$ and $m_k(s)$. It’s hard to believe that a solver would compute $s = 0$ exactly. That suggests that the solver should have stopped earlier.

@MohamedLaghdafHABIBOULLAH Could you please show the run of R2DH where you observed $s = 0$?

@MohamedLaghdafHABIBOULLAH, I agree with Dominique, could you provide us a run where this occurs ?

Sorry I just saw the message, yes of course.

test/test-R2N.jl

dpo

This looks great, thank you! I agree that we can have this PR focus on R2N only, and open future PRs for other solvers.

dpo · 2026-01-27T14:04:02Z

test/test-solver.jl

@@ -0,0 +1,48 @@
+function test_solver(
+  reg_nlp::R,
+  solver_name::String;


Just pass the function name instead of a string.

dpo · 2026-01-27T14:05:03Z

test/test-solver.jl

+  solver_fun = getfield(RegularizedOptimization, Symbol(solver_name))
+  stats_basic = solver_fun(
+    reg_nlp.model,
+    reg_nlp.h,


You should be able to just pass reg_nlp.

dpo · 2026-01-27T14:57:12Z

test/test-solver.jl

+
+  # Remove the x0 entry from solver_kwargs
+  optimized_solver_kwargs = Base.structdiff(solver_kwargs, NamedTuple{(:x0,)})
+  solve!(solver, reg_nlp, stats_optimized; x = x0, optimized_solver_kwargs...) # It would be interesting to check for allocations here as well but depending on 


Testing for allocations would be the main point I think, because the first set of test (where you call solver_fun) already call solver_contructor and solve!(). I wonder if something like @wrappedallocs @eval solve!(solver, reg_nlp, stats; atol = $atol) would work. I agree that allocation tests need not be in this function, but, in general, it should be possible to construct the expression of the solve call and measure allocations.

MaxenceGollier added 5 commits December 18, 2025 11:49

move wrappedallocs macro to utils file

59ba728

add rosenbrock test function

80979c4

add test solver function

33d0800

apply test solver function for R2N

f4ff437

apply JuliaFormatter

0dd0fd8

MaxenceGollier and others added 2 commits January 3, 2026 10:04

Merge branch 'JuliaSmoothOptimizers:master' into update-test

66f8b13

add fixmes for lsr1 allocation tests

80070be

MaxenceGollier added 3 commits January 3, 2026 11:27

add callback test for R2N

1a3e079

mention open PR in fixmes

cbf4ab9

mention divide by 0 in fixme

ad20f48

MaxenceGollier marked this pull request as ready for review January 3, 2026 10:39

remove test from callback and add in test-solver instead

8dcdd0b

MaxenceGollier requested review from MohamedLaghdafHABIBOULLAH and dpo January 3, 2026 10:58

MohamedLaghdafHABIBOULLAH reviewed Jan 4, 2026

View reviewed changes

test/test-R2N.jl Show resolved Hide resolved

MaxenceGollier changed the title ~~[WIP] Refactor Tests~~ Refactor tests: R2N Jan 5, 2026

MaxenceGollier mentioned this pull request Jan 26, 2026

Add options to recompute gradient and/or obj or not on first iteration #280

Merged

Merge branch 'JuliaSmoothOptimizers:master' into update-test

b29b832

dpo reviewed Jan 27, 2026

View reviewed changes

Conversation

MaxenceGollier commented Dec 18, 2025

Uh oh!

MaxenceGollier commented Dec 18, 2025

Uh oh!

MohamedLaghdafHABIBOULLAH commented Dec 23, 2025

Uh oh!

codecov bot commented Jan 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

MaxenceGollier commented Jan 3, 2026

Uh oh!

MohamedLaghdafHABIBOULLAH commented Jan 4, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MaxenceGollier Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MohamedLaghdafHABIBOULLAH Jan 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dpo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Jan 3, 2026 •

edited

Loading

MaxenceGollier Jan 5, 2026 •

edited

Loading

MohamedLaghdafHABIBOULLAH Jan 4, 2026 •

edited

Loading