-
Notifications
You must be signed in to change notification settings - Fork 16
R2NLS solver for unconstrained nonlinear least-squares (NLS) problems
#311
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.
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
base: main
Are you sure you want to change the base?
R2NLS solver for unconstrained nonlinear least-squares (NLS) problems
#311
Conversation
Introduces the R2NLS solver, an inexact second-order quadratic regularization method for nonlinear least-squares, with support for multiple subproblem solvers including QRMumps. Updates documentation, tests, and dependencies to include R2NLS and its utilities, and adds comprehensive test coverage for the new solver and its subsolvers.
The finalizer that calls close! on QRMumpsSolver has been commented out due to crashes in libdqrm.dll during object finalization, likely caused by a double free or invalid pointer reference. Additional minor changes include switching from view to direct indexing for SparseMatrixCOO construction to avoid pointer issues, and reordering some operations in SolverCore.solve! for clarity.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #311 +/- ##
==========================================
+ Coverage 89.00% 89.40% +0.40%
==========================================
Files 7 8 +1
Lines 1200 1435 +235
==========================================
+ Hits 1068 1283 +215
- Misses 132 152 +20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Eliminated a duplicate call to mul!(∇f, Jx', r) in the SolverCore.solve! function to avoid unnecessary computation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @farhadrclass for the PR. I made a first batch of comments (but didn't check the new algorithm)
src/R2NLS.jl
Outdated
| ls_subsolver.irn[1:ls_subsolver.nnzj], | ||
| ls_subsolver.jcn[1:ls_subsolver.nnzj], | ||
| ls_subsolver.val[1:ls_subsolver.nnzj], | ||
| ) #For now till they fix the SparseMatrixCOO to accept pointers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an open issue for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, but @dpo and I thinking about this right now
| SolverCore = "0.3" | ||
| SolverParameters = "0.1" | ||
| SolverTools = "0.9" | ||
| SparseArrays = "1.11.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| SparseArrays = "1.11.0" | |
| SparseArrays = "1.10.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we want 1.10?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deleted utilities.jl and its tests, removing the opnorm/TSVD utilities and all related test coverage. Dropped Arpack and GenericLinearAlgebra from dependencies and compat, and updated code and tests to no longer use or include them. Fixed a typo in the CRLS subsolver dispatch in test_solvers.jl.
|
@tmigot thank you for the review, I applied most of what you recommend and asked some questions to confirm the rest |
Introduces the R2NLSParameterSet struct to encapsulate all algorithmic parameters for the R2NLS solver, replacing individual keyword arguments. Updates R2SolverNLS and R2NLS constructors and solve! logic to use the new parameter set, improving type safety, maintainability, and clarity of parameter handling.
Introduces a ShiftedOperator type for shifted linear systems, updates R2NSolver to support new subsolvers (cg, cr), and refactors subsolver dispatch logic. Adds non-positive curvature handling strategies, improves step selection, and updates logging for more detailed iteration information. Also adds a Project.toml for HSL_jll dependency and minor improvements to R2NLS.jl parameter handling.
Added comprehensive test suite for R2N and its subsolvers in my_R2N_dev/test_R2N.jl. Updated Project.toml files to include new dependencies and compat entries. Refactored R2N.jl to improve ShiftedOperator, subsolver dispatch, and negative curvature handling, and removed HSL_jll usage. Minor documentation and logic improvements throughout.
Uncommented and revised the MA97 solver test in test_R2N.jl, added HSL_jll as a dependency, and improved MA97 integration in R2N.jl. Adjusted Hessian-vector product logic to use hprod! for MA97Solver, and made minor code style and efficiency improvements throughout.
Redesigns MA97Solver to use index mappers for direct updates to the MA97 value buffer, avoiding repeated sparse matrix construction. This improves efficiency by mapping Hessian and diagonal entries directly to their positions in the CSC structure, and updates the subsolve! method accordingly. Also updates ShiftedOperator to support AbstractMatrix and improves opnorm handling for both operator and matrix cases.
Introduces a new test file for R2N error handling and updates existing test suites to include R2N and R2N_exact solvers. This ensures allocation, callback, consistency, restart, and unbounded tests now cover these solvers, improving test coverage and robustness for the R2N family.
Replaces all instances of 'subsolver_type = ShiftedLBFGSSolver' with 'subsolver= :shifted_lbfgs' in test and development files.
Replaces the MA97Solver with a generic HSLDirectSolver that can wrap both MA97 and MA57 HSL direct solvers. Updates all relevant logic, type dispatch, and subsolve! methods to support both solvers, and adds :ma57 as an allowed subsolver. This refactor simplifies the codebase and enables easier extension to other HSL direct solvers.
This pull request introduces the new
R2NLSsolver for unconstrained nonlinear least-squares (NLS) problems, expands documentation and test coverage for this solver.Major features and solver additions:
R2NLSsolver, an inexact second-order quadratic regularization method for unconstrained NLS, to the codebase and exported it for use. (src/JSOSolvers.jl,src/R2NLS.jl,README.md,docs/src/solvers.md) [1] [2] [3] [4]R2NLSas an available solver for unconstrained NLS problems. (docs/src/solvers.md,README.md) [1] [2] [3]Utilities and infrastructure:
opnormutility function inutilities.jlthat computes operator norms for both standard numeric and arbitrary element types, using LAPACK, ARPACK, or TSVD as appropriate. This is now included and exported in the main module. (src/utilities.jl,src/JSOSolvers.jl) [1] [2]opnormutility, covering multiple numeric types and matrix shapes. (test/test_Utilities.jl)Testing enhancements:
R2NLSand its subsolver variants in all relevant test suites, including allocation, callback, consistency, restart, and general solver tests. (test/allocs.jl,test/callback.jl,test/consistency.jl,test/restart.jl,test/runtests.jl,test/test_solvers.jl) [1] [2] [3] [4] [5] [6] [7]Dependency and import updates:
Arpack,SparseArrays, andKrylovin both the main module and test files to support the new solver and utilities. (src/JSOSolvers.jl,test/runtests.jl) [1] [2]Note: