Add matrix_free parameter to make fh compatible with ADNLPModels#75
Conversation
|
Thank you @MohamedLaghdafHABIBOULLAH |
src/fh_model.jl
Outdated
| 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...), |
There was a problem hiding this comment.
Do you also need to bump the version of ADNLPModels?
There was a problem hiding this comment.
It seems so; I’m now trying to ensure that the tests pass locally.
There was a problem hiding this comment.
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 testingHowever 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```
There was a problem hiding this comment.
You can see here that ADNLPModels still with the old 0.7 version.
There was a problem hiding this comment.
I will put back the ancient code to see if the cirrus will pass, which is not the case locally...
|
@dpo As I suspected, the tests fail even though I literally did not change anything compared to master, so it must be related to some recent updates of some packages. |
|
When I compare the One improvement would be to have a |
|
Should I address this issue in a different PR? |
|
Do you think @dpo that this package might be useful https://github.com/JuliaTesting/Aqua.jl? |
|
Actually I tried again and the test passes. I now suspect the failure is due to the random nature of the pertubation in the data. |
|
So I will put back the matrix_free parameter, do you think it's good idea to add ADNLPModels@0.8 as a version in the project.toml in Line 18-19 [compat]
ADNLPModels = "^0.3, 0.4, 0.7, 0.8" |
|
Did you test it? |
|
The same issue occurs: the tests pass only when ADNLPModels and DifferentialEquations are added as direct dependencies; otherwise, they do not. |
Yes, here and in all other packages. |
3 participants
@dpo It seems there is an issue with the newer versions of ADNLPModels. Following @nathanemac’s suggestion, I added a matrix_free parameter to ensure compatibility with the latest version of ADNLPModels.