Rework FH problem: reduce allocations and improve efficiency by MaxenceGollier · Pull Request #92 · JuliaSmoothOptimizers/RegularizedProblems.jl

MaxenceGollier · 2026-02-25T19:10:56Z

closes #91
partially closes #90 by adding the OrdinaryDiffEqVerner dep instead of the much much much larger DifferentialEquations dep.
I will use this for the last problem of #88, with NLPModelsModifiers.jl.
I removed the MLDataSet from the dep because I had compat issues. I am waiting for chengchingwen/Pickle.jl#47 to add it back.

I think nonetheless that having to download a large dataset on each CI run is not a good idea and should be avoided (50 minutes of CI for a package of this scale is ridiculous).

@MohamedLaghdafHABIBOULLAH, this is breaking i would like to see your review on this one and make sure that this does not destroy to much code. Can we meet at GERAD sometime ?

codecov · 2026-02-25T19:18:34Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.75%. Comparing base (85de184) to head (838b360).
⚠️ Report is 35 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main      #92       +/-   ##
===========================================
- Coverage   99.18%   77.75%   -21.44%     
===========================================
  Files          11       19        +8     
  Lines         369      481      +112     
===========================================
+ Hits          366      374        +8     
- Misses          3      107      +104

☔ 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-02-25T22:17:05Z

For example, with this version

julia> model, _, _ = fh_model()
julia> @benchmark grad!(model, gx, model.meta.x0)
BenchmarkTools.Trial: 232 samples with 1 evaluation per sample.
 Range (min … max):  20.801 ms …  25.364 ms  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     21.283 ms               ┊ GC (median):    0.00%
 Time  (mean ± σ):   21.518 ms ± 737.232 μs  ┊ GC (mean ± σ):  0.00% ± 0.00%

      ▄██▁▂▁▄▃
  ▃▅▇▅████████▅▆▅▅▃▃▄▄▃▄▁▃▃▁▁▃▃▁▁▃▃▁▃▁▁▃▁▁▃▃▁▃▃▃▃▃▃▁▁▃▃▁▃▁▁▁▃▃ ▃
  20.8 ms         Histogram: frequency by time         24.1 ms <

 Memory estimate: 624 bytes, allocs estimate: 3.

I am trying to get results with the older version but i am running in a ton of problems, it seems that the previous version was not even compatible with ADNLPModels@v0.8.

MohamedLaghdafHABIBOULLAH · 2026-02-26T19:20:11Z

@MaxenceGollier this seems to be extraordinary, I will review it ASAP, yes sure we can meet!

dpo · 2026-03-05T19:50:51Z

@MaxenceGollier Could you please check that objective, gradients, etc., match the old implementation?

Copilot

Pull request overview

This PR refactors the Fitzhugh–Nagumo (FH) problem implementation to reduce allocations and dependency weight by switching from DifferentialEquations to OrdinaryDiffEqVerner, and by using ReverseDiff-based AD backends.

Changes:

Replace DifferentialEquations usage with OrdinaryDiffEqVerner and introduce ReverseDiff for FH objective/residual evaluation.
Rework FH residual/objective to use in-place ODE solves with preallocated buffers.
Update dependencies/compat and documentation to reflect the new required packages for FH.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`test/runtests.jl`	Switch test deps to `OrdinaryDiffEqVerner`; SVM test blocks are currently wrapped in a triple-quoted string (disabled).
`src/RegularizedProblems.jl`	Gate FH model loading behind `Requires` on `OrdinaryDiffEqVerner` + `ReverseDiff`.
`src/fh_model.jl`	Major FH rewrite: integrator reuse, in-place residual, ReverseDiff backends.
`Project.toml`	Remove `DifferentialEquations` from test extras/targets; add `OrdinaryDiffEqVerner` + `ReverseDiff`.
`docs/src/index.md`	Document updated imports required for `fh_model()`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/runtests.jl

src/fh_model.jl

github-actions · 2026-04-10T18:28:16Z

Package name	latest	stable
RegularizedOptimization

github-actions · 2026-04-10T18:29:40Z

Package name	latest	stable
RegularizedOptimization

github-actions · 2026-04-10T18:52:57Z

Package name	latest	stable
RegularizedOptimization

github-actions · 2026-04-10T18:53:37Z

Package name	latest	stable
RegularizedOptimization

github-actions · 2026-04-13T13:26:42Z

Package name	latest	stable
RegularizedOptimization

MaxenceGollier · 2026-04-13T15:41:18Z

@dpo, @MohamedLaghdafHABIBOULLAH.
(cc. @BenjaminPINEAU)
Let's try to merge this this week ?

1) Time for CI to complete

from ~50 minutes to ~15 minutes :)

2) Matching the old implementation

The result don't match exactly, see explanation below

This version:

using ADNLPModels, LinearAlgebra, NLPModels, OrdinaryDiffEqVerner, Random, RegularizedProblems, ReverseDiff 

Random.seed!(0)

nlp, nls, _ = fh_model()

println("NLP model...")
# x0 = [1, 1, 1, 1, 1]
println("f(x0) = $(obj(nlp, nlp.meta.x0))")
println("∇f(x0) = $(grad(nlp, nlp.meta.x0))")

x_star = [0, 0.2, 1, 0, 0]
println("f(x*) = $(obj(nlp, x_star))")
println("∇f(x*) = $(grad(nlp, x_star))")

println("NLS model...")
# x0 = [1, 1, 1, 1, 1]
println("f(x0) = $(obj(nls, nlp.meta.x0))")
println("∇f(x0) = $(grad(nls, nlp.meta.x0))")

x_star = [0, 0.2, 1, 0, 0]
println("f(x*) = $(obj(nls, x_star))")
println("∇f(x*) = $(grad(nls, x_star))")


NLP model...
f(x0) = 197.95619423692332
∇f(x0) = [1356.6673288925638, -431.3353610583008, 223.18656497528306, 799.2501203790903, -1258.6056018037166]
f(x*) = 0.9770409013057835
∇f(x*) = [2.412145383320563, -15.08519167121669, -4.362149256446968, 1.5667814395376296, -1.8221806391545878]
NLS model...
f(x0) = 197.95619423692332
∇f(x0) = [1356.667328892572, -431.3353610583022, 223.18656497528292, 799.2501203790955, -1258.6056018037239]
f(x*) = 0.9770409013057835
∇f(x*) = [2.412153852028672, -15.085273625467737, -4.362184181342515, 1.5667841023129923, -1.822184174767189]

On v0.1.5:

using ADNLPModels, DifferentialEquations, NLPModels, Random, RegularizedProblems

Random.seed!(0)

nlp, nls, _ = fh_model()

println("NLP model...")
# x0 = [1, 1, 1, 1, 1]
println("f(x0) = $(obj(nlp, nlp.meta.x0))")
println("∇f(x0) = $(grad(nlp, nlp.meta.x0))")

x_star = [0, 0.2, 1, 0, 0]
println("f(x*) = $(obj(nlp, x_star))")
println("∇f(x*) = $(grad(nlp, x_star))")

println("NLS model...")
# x0 = [1, 1, 1, 1, 1]
println("f(x0) = $(obj(nls, nlp.meta.x0))")
println("∇f(x0) = $(grad(nls, nlp.meta.x0))")

x_star = [0, 0.2, 1, 0, 0]
println("f(x*) = $(obj(nls, x_star))")
println("∇f(x*) = $(grad(nls, x_star))")

NLP model...
f(x0) = 198.19152510098036
∇f(x0) = [1347.7950070522143, -429.80028576306523, 223.97176160928552, 789.974932231761, -1248.906457695463]
f(x*) = 0.9770387293264109
∇f(x*) = [-5.470921114056369, 84.87444452657626, 22.037203595006023, -0.9650077346427708, 2.9605611066693323]
NLS model...
f(x0) = 198.19152510098036
∇f(x0) = [1347.7950070522131, -429.80028576306245, 223.97176160928709, 789.9749322317585, -1248.9064576954622]
f(x*) = 0.9770387293264109
∇f(x*) = [-5.470921114057837, 84.87444452660499, 22.037203595014066, -0.9650077346431544, 2.9605611066703648]

I was able to check that this is due to the fact that the two solve implementations from DifferentialEquations do not give exactly the same answer.
The first step computed by the ODE solver from my implementation when I call f(x0) produces [2.003611804347596, 0.5458089065499695] while the call from the v0.1.5 produces [2.003611804347596, 0.5458089065499702], this accumulates over the time steps. I wasn't able to find why we can not get the exact same answer.

3) Speed and Memory

This version:

using ADNLPModels, BenchmarkTools, LinearAlgebra, NLPModels, OrdinaryDiffEqVerner, Random, RegularizedProblems, ReverseDiff 

Random.seed!(0)

nlp, _, _ = fh_model()

@benchmark grad(nlp, nlp.meta.x0)
BenchmarkTools.Trial: 182 samples with 1 evaluation per sample.
 Range (min … max):  25.890 ms … 41.764 ms  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     26.920 ms              ┊ GC (median):    0.00%
 Time  (mean ± σ):   27.443 ms ±  1.657 ms  ┊ GC (mean ± σ):  0.00% ± 0.00%

     ▂ ██▇▃▇▅▁                                                 
  ▇▆▆██████████▅▅▄▇▃▃▄▅▅▃▄▃▆▃▄▅▅▁▃▄▅▃▃▃▁▁▃▃▁▄▅▃▁▁▁▁▁▃▁▁▁▁▁▁▁▃ ▃
  25.9 ms         Histogram: frequency by time        32.1 ms <

 Memory estimate: 704 bytes, allocs estimate: 4.

On v0.1.5:

using ADNLPModels, BenchmarkTools, DifferentialEquations, NLPModels, Random, RegularizedProblems

Random.seed!(0)

nlp, _, _ = fh_model()

@benchmark grad(nlp, nlp.meta.x0)
BenchmarkTools.Trial: 4087 samples with 1 evaluation per sample.
 Range (min … max):  652.243 μs … 122.746 ms  ┊ GC (min … max):  0.00% … 99.23%
 Time  (median):     741.151 μs               ┊ GC (median):     0.00%
 Time  (mean ± σ):     1.221 ms ±   3.196 ms  ┊ GC (mean ± σ):  22.81% ± 11.45%

  █▂▁▅▅                                                         ▁
  █████▄▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▃▇ █
  652 μs        Histogram: log(frequency) by time       13.4 ms <

 Memory estimate: 1.89 MiB, allocs estimate: 30076.

The older version has some faster runs and some are really worse, the newer has a smaller range of time values and it allocates less.

github-actions · 2026-04-13T16:02:53Z

Package name	latest	stable
RegularizedOptimization

MaxenceGollier added 6 commits April 10, 2026 14:13

reduce allocations in FH problem

591f68d

remove DifferentialEquations dep and replace with OrdinaryDiffEq

f183a0a

rework fh model

418ea43

update doc

62ba149

revert changes to Project.toml

57f253d

revert changes to Project.toml 2

5e54e3d

MaxenceGollier force-pushed the pde_fh branch from c0c4552 to 5e54e3d Compare April 10, 2026 18:15

MaxenceGollier marked this pull request as ready for review April 10, 2026 18:15

Copilot AI review requested due to automatic review settings April 10, 2026 18:15

Copilot started reviewing on behalf of MaxenceGollier April 10, 2026 18:16 View session

revert project.toml 3

d69f973

Copilot AI reviewed Apr 10, 2026

View reviewed changes

test/runtests.jl Show resolved Hide resolved

test/runtests.jl Outdated Show resolved Hide resolved

src/fh_model.jl Show resolved Hide resolved

src/fh_model.jl Outdated Show resolved Hide resolved

src/fh_model.jl Show resolved Hide resolved

MaxenceGollier added 3 commits April 10, 2026 14:30

add explicit compat to DataStructures

25fd40b

fix project.toml 4

a728337

revert MLDatasets tests

940bc91

update compat

d7c8d62

apply suggestions from copilot

838b360

Conversation

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

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

MaxenceGollier commented Feb 25, 2026

Uh oh!

MohamedLaghdafHABIBOULLAH commented Feb 26, 2026

Uh oh!

dpo commented Mar 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Apr 10, 2026

Uh oh!

github-actions bot commented Apr 10, 2026

Uh oh!

github-actions bot commented Apr 10, 2026

Uh oh!

github-actions bot commented Apr 10, 2026

Uh oh!

github-actions bot commented Apr 13, 2026

Uh oh!

MaxenceGollier commented Apr 13, 2026

1) Time for CI to complete

2) Matching the old implementation

3) Speed and Memory

Uh oh!

github-actions bot commented Apr 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MaxenceGollier commented Feb 25, 2026 •

edited

Loading

codecov bot commented Feb 25, 2026 •

edited

Loading