add ScaledModel - #123
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #123 +/- ##
==========================================
- Coverage 97.29% 92.87% -4.42%
==========================================
Files 6 7 +1
Lines 886 1067 +181
==========================================
+ Hits 862 991 +129
- Misses 24 76 +52 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
| the gradient and the Jacobian evaluated at the initial point ``x0``. | ||
|
|
||
| """ | ||
| struct ScaledModel{T, S, M} <: NLPModels.AbstractNLPModel{T, S} |
There was a problem hiding this comment.
and same comment throughout the file
|
The linear and nonlinear API for the constraints have been implemented. |
dpo
left a comment
dpo
left a comment
There was a problem hiding this comment.
Thank you! I think we can use this in multiple places. I just have a few comments to make the code more explicit.
| # Compute scaling as min(1, max_gradient / norm(∇cᵢ, Inf) ) | ||
| for i in eachindex(cons) | ||
| cons[i] = min(1.0, max_gradient / cons[i]) | ||
| end |
There was a problem hiding this comment.
Either I don't understand this function, or it doesn't do what's announced above. Maybe a docstring would clarify.
There was a problem hiding this comment.
The idea is to first compute the norm inf of the gradient of each constraint in cons. Then, the scaling is defined as specified in the comment.
| end | ||
| end | ||
|
|
||
| function _set_jacobian_scaling!(Jx, Ji, Jj, cons) |
There was a problem hiding this comment.
The name cons suggests "constraint" (values). But that's not what it is, is it?
There was a problem hiding this comment.
Indeed, the name scaling is more appropriate
Co-authored-by: Maxence Gollier <134112149+MaxenceGollier@users.noreply.github.com>
4 participants
Following a suggestion by @dpo