Separate default implementations from api.jl #513

arnavk23 · 2025-10-03T10:21:55Z

Create src/nlp/defaults.jl for default implementations
Update src/NLPModels.jl to include defaults.jl after api.jl
Move default implementations from api.jl to defaults.jl
Replace implementations with function stubs in api.jl
Resolves method overwriting warnings during precompilation

Functions refactored:

grad(), cons(), cons!(), cons_lin(), cons_nln()
jth_congrad(), objcons(), objcons!(), objgrad(), objgrad!()
jac_structure(), jac_structure!(), jac_lin_structure(), jac_nln_structure()

Closes #385

- Create src/nlp/defaults.jl for default implementations - Update src/NLPModels.jl to include defaults.jl after api.jl - Move default implementations from api.jl to defaults.jl - Replace implementations with function stubs in api.jl - Resolves method overwriting warnings during precompilation - Fixes GitHub issue JuliaSmoothOptimizers#385: Separate the default implementations from the api file Functions refactored: - grad(), cons(), cons!(), cons_lin(), cons_nln() - jth_congrad(), objcons(), objcons!(), objgrad(), objgrad!() - jac_structure(), jac_structure!(), jac_lin_structure(), jac_nln_structure() All tests pass and module loads without warnings.

arnavk23 · 2025-10-03T13:32:40Z

@tmigot @amontoison please review this pr.

tmigot

Thanks @arnavk23 for the PR. Are you planning to do the same for NLS in a separate PR?

src/NLPModels.jl

src/nlp/defaults.jl

Co-authored-by: Tangi Migot <tangi.migot@gmail.com>

src/nlp/defaults.jl

src/NLPModels.jl

Co-authored-by: Tangi Migot <tangi.migot@gmail.com>

dpo · 2025-10-04T17:57:16Z

Functions refactored:

@arnavk23 Could you please explain what you refactored exactly?

arnavk23 · 2025-10-05T05:36:01Z

Could you please explain what you refactored exactly?

@dpo

Files changed / added

src/NLPModels.jl
- Updated includes to load the new defaults files after API files:
  - Added include("nlp/defaults.jl")
  - Added include("nls/defaults.jl")
- Reason: ensure API declarations are loaded first, then default implementations.
src/nlp/api.jl
- Replaced concrete method bodies with documentation + minimal stubs function foo end for many API entry points (keeps the docs while preventing duplicate definitions).
src/nlp/defaults.jl (new)
- Added the default implementations for NLP model APIs (the functions moved out of src/nlp/api.jl).
- Purpose: provide fallback behavior for models that don’t implement specialized methods.
src/nls/api.jl
- Replaced concrete method bodies with stubs for the NLS API (preserved docs).
src/nls/defaults.jl (new)
- Added default implementations for NLS APIs (moved out of src/nls/api.jl).

Concrete functions moved (representative list)

NLP (moved into src/nlp/defaults.jl)

Objective/constraint helpers: obj, obj!, objcons, objcons!, objgrad, objgrad!
Gradients / constraints: grad, grad!, cons, cons!
Jacobian helpers & structure: jac_structure, jac_structure!, jac_lin_structure, jac_nln_structure, jac_coord, jac, jac_lin_coord, jac_nln_coord
Jacobian products: jprod, jprod_lin, jprod_nln, jtprod, jtprod_lin, jtprod_nln
Operator forms: jac_op, jac_lin_op, jac_nln_op
Hessian helpers: jth_hess_coord, jth_hess, jth_hprod, ghjvprod

NLS (moved into src/nls/defaults.jl)

Residual & Jacobian: residual, residual!, jac_residual, jac_structure_residual, jac_structure_residual!, jac_coord_residual, jac_coord_residual!
Jacobian products: jprod_residual, jprod_residual!, jtprod_residual, jtprod_residual!
Jacobian operator: jac_op_residual, jac_op_residual!
Hessian/residual Hessian helpers: hess_residual, hess_structure_residual, hess_structure_residual!, hess_coord_residual, hess_coord_residual!, jth_hess_residual_coord!, hprod_residual!, hess_op_residual!
Objective/grad helpers adapted to NLS: obj, objcons!, grad!, objgrad!

Why this change fixes the problem

Julia throws an error during precompilation when a method is defined more than once across files that are included during precompilation time.
Moving implementations into defaults.jl and leaving api.jl as declarations prevents duplicate definitions (the API declares the name; the defaults file defines the fallback once).
Including defaults after API files in src/NLPModels.jl ensures API declarations exist before default implementations are loaded.

src/nlp/defaults.jl

src/nls/defaults.jl

src/nls/api.jl

Co-authored-by: Tangi Migot <tangi.migot@gmail.com>

src/nls/api.jl

Copilot

Pull request overview

This PR refactors the NLPModels codebase by separating default function implementations from API definitions. It creates new defaults.jl files for both NLP and NLS models, moves default implementations from api.jl to these new files, and replaces the moved implementations with function stubs in api.jl. This restructuring aims to resolve method overwriting warnings during precompilation.

Key Changes:

Created src/nlp/defaults.jl and src/nls/defaults.jl to house default implementations
Modified src/NLPModels.jl to include defaults.jl files after corresponding api.jl files
Converted select function implementations in api.jl files to stubs (bare function declarations)

Reviewed changes

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

Show a summary per file

File	Description
src/NLPModels.jl	Updated file inclusion list to add "defaults" between "api" and "counters"
src/nlp/api.jl	Replaced selected function implementations with stubs; added NLS-specific documentation for obj() and objcons!()
src/nlp/defaults.jl	New file containing default implementations for grad, cons, cons!, cons_lin, cons_nln, jth_congrad, objcons, objcons!, objgrad, objgrad!, jac_structure, jac_structure!, jac_lin_structure, jac_nln_structure, and various jacobian/hessian operator functions
src/nls/api.jl	Replaced function implementations with stubs for residual, jacobian, and hessian functions specific to NLS models
src/nls/defaults.jl	New file containing default implementations for NLS-specific functions including residual, jac_residual, jac_op_residual, hess_residual, obj, grad!, objcons!, and objgrad!

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

src/nlp/api.jl

src/nlp/defaults.jl

Copilot · 2025-12-13T09:53:57Z

src/nlp/defaults.jl

+function jtprod_lin(nlp::AbstractNLPModel{T, S}, v::AbstractVector) where {T, S}
+  @lencheck nlp.meta.nlin v
+  Jtv = S(undef, nlp.meta.nvar)
+  return jtprod_lin!(nlp, v, Jtv)
+end


Critical signature mismatch: This function is defined without the x parameter, but code in api.jl (lines 509-514 and within implementations at lines 469, 473, 480, 688) expects signatures that include x like jtprod_lin(nlp, x, v) and jtprod_lin!(nlp, x, v, Jtv). Since linear constraints don't depend on x, the signature without x is theoretically correct, but this creates a breaking change where existing API calls will fail with "MethodError: no method matching" errors. Either defaults.jl needs to add x parameter to match existing API, or api.jl needs to be updated to remove x from all linear constraint function calls.

Copilot · 2025-12-13T09:53:57Z

src/nlp/defaults.jl

+function jac_lin_op(nlp::AbstractNLPModel{T, S}) where {T, S}
+  Jv = S(undef, nlp.meta.nlin)
+  Jtv = S(undef, nlp.meta.nvar)
+  return jac_lin_op!(nlp, Jv, Jtv)
+end
+
+function jac_lin_op!(
+  nlp::AbstractNLPModel{T, S},
+  Jv::AbstractVector,
+  Jtv::AbstractVector,
+) where {T, S}
+  @lencheck nlp.meta.nlin Jv
+  @lencheck nlp.meta.nvar Jtv
+  prod! = @closure (res, v, α, β) -> begin # res = α * J * v + β * res
+    jprod_lin!(nlp, v, Jv)
+    if β == 0
+      res .= α .* Jv
+    else
+      res .= α .* Jv .+ β .* res
+    end
+    return res
+  end
+  ctprod! = @closure (res, v, α, β) -> begin
+    jtprod_lin!(nlp, v, Jtv)
+    if β == 0
+      res .= α .* Jtv
+    else
+      res .= α .* Jtv .+ β .* res
+    end
+    return res
+  end
+  return LinearOperator{T}(nlp.meta.nlin, nlp.meta.nvar, false, false, prod!, ctprod!, ctprod!)
+end


Critical signature mismatch: This function is defined without the x parameter (line 489: jac_lin_op(nlp) and line 495: jac_lin_op!(nlp, Jv, Jtv)), but code in api.jl (lines 655-660, 670-697) expects signatures that include x like jac_lin_op(nlp, x) and jac_lin_op!(nlp, x, Jv, Jtv). This creates a signature mismatch where the api.jl wrappers with x parameter won't correctly delegate to these implementations. Either add the x parameter here to match the API, or update api.jl to remove x from linear Jacobian operator functions.

src/nlp/api.jl

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

tmigot requested changes Oct 3, 2025

View reviewed changes

src/NLPModels.jl Outdated Show resolved Hide resolved

src/NLPModels.jl Outdated Show resolved Hide resolved

src/nlp/defaults.jl Outdated Show resolved Hide resolved

arnavk23 and others added 3 commits October 4, 2025 06:54

Update src/NLPModels.jl

d826ab7

Co-authored-by: Tangi Migot <tangi.migot@gmail.com>

Update NLPModels.jl

eda7309

Update defaults.jl

3097564

arnavk23 requested a review from tmigot October 4, 2025 03:42

Update NLPModels.jl

844ef56

tmigot requested changes Oct 4, 2025

View reviewed changes

src/nlp/defaults.jl Outdated Show resolved Hide resolved

src/NLPModels.jl Outdated Show resolved Hide resolved

arnavk23 and others added 2 commits October 4, 2025 18:57

Update src/nlp/defaults.jl

b1aedea

Co-authored-by: Tangi Migot <tangi.migot@gmail.com>

Add NLS defaults and move default implementations out of nls/api.jl

57e330e

arnavk23 requested a review from tmigot October 4, 2025 15:55

tmigot requested changes Oct 15, 2025

View reviewed changes

arnavk23 and others added 5 commits October 15, 2025 17:10

Update src/nls/defaults.jl

46c2357

Co-authored-by: Tangi Migot <tangi.migot@gmail.com>

Update src/nls/api.jl

c272e8a

Co-authored-by: Tangi Migot <tangi.migot@gmail.com>

Update src/nls/api.jl

a33c13d

Co-authored-by: Tangi Migot <tangi.migot@gmail.com>

Update src/nls/api.jl

84e156e

Co-authored-by: Tangi Migot <tangi.migot@gmail.com>

Add new NLP API with separate defaults

417b471

arnavk23 requested a review from tmigot October 15, 2025 18:45

tmigot requested changes Oct 18, 2025

View reviewed changes

src/nls/api.jl Outdated Show resolved Hide resolved

resturucturing API defaults into separate files

c54b329

arnavk23 requested a review from tmigot October 18, 2025 12:02

Merge branch 'main' into separate-api-defaults

df4e872

Copilot AI review requested due to automatic review settings December 13, 2025 09:45

Copilot started reviewing on behalf of arnavk23 December 13, 2025 09:45 View session

Copilot AI reviewed Dec 13, 2025

View reviewed changes

Apply suggestions from code review

2bc1bde

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Separate default implementations from api.jl #513

Are you sure you want to change the base?

Separate default implementations from api.jl #513

Uh oh!

Conversation

arnavk23 commented Oct 3, 2025

Uh oh!

arnavk23 commented Oct 3, 2025

Uh oh!

tmigot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dpo commented Oct 4, 2025

Uh oh!

arnavk23 commented Oct 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Files changed / added

Concrete functions moved (representative list)

Why this change fixes the problem

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes:

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

arnavk23 commented Oct 5, 2025 •

edited

Loading