Skip to content

Manual NLSModel - #37

Merged
dpo merged 6 commits into
mainfrom
manual-nls
Aug 12, 2025
Merged

Manual NLSModel#37
dpo merged 6 commits into
mainfrom
manual-nls

Conversation

@dpo

@dpo dpo commented Jul 27, 2025

Copy link
Copy Markdown
Member

No description provided.

@dpo

dpo commented Jul 27, 2025

Copy link
Copy Markdown
Member Author

Needs more unit tests.

@dpo
dpo marked this pull request as draft July 27, 2025 17:56
@dpo
dpo requested a review from Copilot July 27, 2025 17:57

This comment was marked as outdated.

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

@tmigot tmigot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @dpo ! That looks like a great add to this package.

Comment thread src/nls_api.jl
Comment thread test/notimplemented.jl Outdated
@dpo
dpo requested a review from Copilot August 11, 2025 15:56

This comment was marked as outdated.

@dpo
dpo marked this pull request as ready for review August 11, 2025 18:14
@dpo
dpo requested a review from tmigot August 11, 2025 18:15

@tmigot tmigot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @dpo ! That's a really great add.

Comment thread src/structure.jl
Comment thread test/nlpmodelstest.jl
# jth_hess_residual_coord,
hprod_residual,
hess_op_residual,
]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Did you try adding the "Check consistency" in a similar way as for nlps?
I guess because this doesn't have the constraints you would have to exclude almost all the functions... So, maybe we should create an issue as a possible follow-up of this PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don’t understand that test; it seems to be comparing a model to itself.

@dpo
dpo requested a review from Copilot August 12, 2025 12:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces manual NLS (Nonlinear Least Squares) model functionality to complement the existing NLP model capabilities. The implementation provides a way to define NLS problems manually by specifying residual functions and their derivatives.

Key changes:

  • Adds NLSModel struct and constructor for creating least squares optimization problems
  • Implements NLS-specific API functions for residual evaluation and Jacobian operations
  • Includes comprehensive test problems (MGH01 and MGH04) demonstrating the new functionality

Reviewed Changes

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

Show a summary per file
File Description
src/structure.jl Exports NLSModel and defines the main NLSModel struct with constructor
src/nls_api.jl Implements NLS-specific API functions for residual and Jacobian operations
src/ManualNLPModels.jl Includes the new NLS API module in the package
test/problems/mgh01.jl Adds MGH01 test problem implementation with Rosenbrock-like residuals
test/problems/mgh04.jl Adds MGH04 test problem implementation with exponential residuals
test/nlpmodelstest.jl Integrates NLS problems into the test suite with precision and dimension checks
test/notimplemented.jl Adds tests verifying proper error handling for unimplemented NLS methods

Comment thread src/structure.jl
Comment thread src/structure.jl Outdated
Comment thread src/structure.jl Outdated
Comment thread test/notimplemented.jl Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@dpo
dpo merged commit d10acc5 into main Aug 12, 2025
13 checks passed
@dpo
dpo deleted the manual-nls branch August 12, 2025 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants