Skip to content

Add matrix_free parameter to make fh compatible with ADNLPModels #75

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add matrix_free parameter to make fh compatible with ADNLPModels #75

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f668048
Add matrix_free parameter to make fh compatible with recent versions …
MohamedLaghdafHABIBOULLAH Sep 12, 2025
bdb0d31
put back the definition of fh_model
MohamedLaghdafHABIBOULLAH Sep 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Manifest.toml
4 changes: 2 additions & 2 deletions src/fh_model.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ of an `ADNLSModel` that represents the same problem, and the exact solution.
function fh_model(; kwargs...)
data, simulate, resid, misfit, x0 = FH_smooth_term()
nequ = 202
ADNLPModels.ADNLPModel(misfit, ones(5); kwargs...),
ADNLPModels.ADNLSModel(resid, ones(5), nequ; kwargs...),
ADNLPModels.ADNLPModel(misfit, ones(5); matrix_free = true, kwargs...),
ADNLPModels.ADNLSModel(resid, ones(5), nequ; matrix_free = true, kwargs...),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you also need to bump the version of ADNLPModels?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems so; I’m now trying to ensure that the tests pass locally.

Copy link
Collaborator Author

@MohamedLaghdafHABIBOULLAH MohamedLaghdafHABIBOULLAH Sep 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's bizarre locally, normally it should work even with the old ADNLPModels versions, however if I test the package as following:

(RegularizedProblems) pkg> activate .
(RegularizedProblems) pkg> instantiate
(RegularizedProblems) pkg> test
Test Summary: | Error  Total   Time
FH            |     1      1  18.6s
ERROR: LoadError: Some tests did not pass: 0 passed, 0 failed, 1 errored, 0 broken.
in expression starting at /Users/laghdafhacen/Documents/GitHub/RegularizedProblems.jl/test/runtests.jl:26
ERROR: Package RegularizedProblems errored during testing

However if I add both ADNLPModels and DifferentialEquations as dependancies of RegularizedProblems the tests pass:

(RegularizedProblems) pkg> add ADNLPModels
   Resolving package versions...
      Compat entries added for 
  No Changes to `~/Documents/GitHub/RegularizedProblems.jl/Project.toml`
  No Changes to `~/Documents/GitHub/RegularizedProblems.jl/Manifest.toml`

(RegularizedProblems) pkg> add DifferentialEquations
   Resolving package versions...
      Compat entries added for 
  No Changes to `~/Documents/GitHub/RegularizedProblems.jl/Project.toml`
  No Changes to `~/Documents/GitHub/RegularizedProblems.jl/Manifest.toml`

(RegularizedProblems) pkg> test
     Testing RegularizedProblems
      Status `/private/var/folders/sy/rzn9bw2d65b2y46rm6lzjx3w0000gn/T/jl_vmN4mu/Project.toml`
⌅ [54578032] ADNLPModels v0.7.2
  [0c46a032] DifferentialEquations v7.16.1
  [31c24e10] Distributions v0.25.120
  [eb30cadb] MLDatasets v0.7.18
  [30dfa513] ManualNLPModels v0.2.0
  [a4795742] NLPModels v0.21.5
⌅ [81d43f40] Noise v0.2.2
⌅ [a725b495] ProximalOperators v0.15.3
  [f468eda6] QuadraticModels v0.9.13
  [ea076b23] RegularizedProblems v0.1.2 `~/Documents/GitHub/RegularizedProblems.jl`
  [ae029012] Requires v1.3.1
  [d4fd37fa] ShiftedProximalOperators v0.2.2
  [37e2e46d] LinearAlgebra v1.11.0
  [9a3f8284] Random v1.11.0
  [2f01184e] SparseArrays v1.11.0
  [8dfed614] Test v1.11.0
      Status `/private/var/folders/sy/rzn9bw2d65b2y46rm6lzjx3w0000gn/T/jl_vmN4mu/Manifest.toml`
⌅ [54578032] ADNLPModels v0.7.2
...
  [0c46a032] DifferentialEquations v7.16.1
  
       Testing Running tests...
Test Summary: | Pass  Total   Time
FH            |    2      2  16.1s
     Testing RegularizedProblems tests passed```

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can see here that ADNLPModels still with the old 0.7 version.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will put back the ancient code to see if the cirrus will pass, which is not the case locally...

x0
end
Loading