Levenberg-Marquart + SLSQP for Proximity #394

jzwar · 2024-03-11T14:00:22Z

Overview

Adds Levenberg-Marquart and SLSQP for proximity. Current implementation wouldn't work for some situations (see #389). Source for SLSQP is from current scipy's fortran version. To avoid difficulties of fortran-compiler-setup in CI systems, the source is translated to c using f2c. Then, removed some static variables and add const keywords.

Now, the search looks like this:

Initial guess
|
Newton
|
Converged? -- yes -- return
|
No
|
Reset initial guess, max_iterations *= 5
|
SLSQP
|
Converged? -- yes -- return
|
No
|
Reset initial guess, max_iterations *= 5
|
LM

Also, SLSQP can incorporate bounds. Nice.

Addressed Issues

fixes Issues with Proximity #389
Sometimes, newton iterations does not converge
Sometimes, we needed to sample a lot of points for k-d tree
Sometimes, we really want to get correct values at any cost.

Checklists

Documentations are up-to-date.
Added example(s)
Added test(s)

j042 · 2024-03-11T16:07:32Z

super cool!

j042 · 2024-04-26T13:27:46Z

Do you have scripts/notes of the origin of this implementation?

j042 · 2024-04-26T15:15:19Z

never mind, found it in #389

j042 · 2024-04-26T15:21:54Z

one thing to note in general is that we probably had to call system.ResetRowOrder(); at each iteration.

j042 · 2024-04-26T15:22:54Z

one thing to note in general is that we probably had to call system.ResetRowOrder(); at each iteration.

On the other hand, it would have iterated through this order and reordered it, so maybe it is irrelevant

j042

Awesome addition! I think we can set Newton max_iter a bit lower than LM. I've added some discussion points.

src/proximity/proximity.cpp

j042 · 2024-04-26T15:38:33Z

src/proximity/proximity.cpp

+    if (modified_marquart) {
+      lhs(i, i) = 2 * spline_gradient_AAt(i, i) * (1 + lambda);
+    } else {
+      lhs(i, i) = 2 * (spline_gradient_AAt(i, i) + lambda);


@jzwar current J removes this 2 *, but can you double check this? With 2*, I thought it should be:

Suggested change

lhs(i, i) = 2 * (spline_gradient_AAt(i, i) + lambda);

lhs(i, i) = (2 * (spline_gradient_AAt(i, i)) + lambda;

j042 · 2024-04-26T15:40:24Z

src/proximity/proximity.cpp

+
+    // compute || F(x + dX) ||^2 then rho
+    ComputeCostAndDerivatives(data, 0); // computes J and everything on the way
+    const double rho =


@jzwar you think it is crucial to keep both neu and alt metrics?

src/proximity/proximity.cpp

License notice included in src commit: 5a6f935 https://github.com/scipy/scipy/blob/main/scipy/optimize/slsqp/slsqp_optmz.f

I have no problem, but codespell does

Initial file created with f2c (https://www.netlib.org/f2c/). Modified to be standalone (without f2c.h)

slsqp for proximity

j042 · 2024-05-03T10:30:21Z

If you get a chance, I'd appreciate if you can take a look at proximity.hpp / proximity.cpp @danielwolff1 :)

danielwolff1

I took a look at it and have some minor comments.

I have to admit that I did not check every single line of code in detail, but I took a few hours to check closely and to me it looks good, I didn't spot anything that seems completely wrong. Therefore, I approve! 🚀

include/splinepy/proximity/proximity.hpp

src/proximity/proximity.cpp

Co-authored-by: Daniel Wolff <[email protected]>

main return first, then additional ones. This applies to GuessMinusQuery, J, dJdu, d2Jdu2

danielwolff1 · 2024-05-04T16:33:40Z

Thanks for incorporating the changes, from my point of view this can be merged! 🚀

Queries outside of the spline sometimes does not return 0.0 convergence_norm. However, slsqp exits with 0 if J itself converges.

j042 · 2024-05-06T14:12:53Z

Thanks for the review @danielwolff1 !

j042 force-pushed the ft-LM-test branch 2 times, most recently from adf8c3a to 5fed186 Compare April 16, 2024 06:47

jzwar added 3 commits April 18, 2024 14:22

First Draft LM

f816169

Try a different metric

712a887

Play around with factors

b7967f2

j042 force-pushed the ft-LM-test branch from 5fed186 to b7967f2 Compare April 18, 2024 12:22

add new layouts for proximity

560c126

reorganize proximity first draft

bcbe36a

j042 force-pushed the ft-LM-test branch from 1596773 to bcbe36a Compare April 26, 2024 15:26

fix warning

82bcc38

j042 reviewed Apr 26, 2024

View reviewed changes

j042 marked this pull request as ready for review April 26, 2024 16:16

j042 added a commit that referenced this pull request Apr 26, 2024

finalize #394

9e8b480

j042 mentioned this pull request Apr 26, 2024

finalize #394 #423

Merged

j042 added 11 commits May 2, 2024 20:21

finalize #394

c384323

auto max_iter tune

e5a0195

source from scipy

6d2c3db

License notice included in src commit: 5a6f935 https://github.com/scipy/scipy/blob/main/scipy/optimize/slsqp/slsqp_optmz.f

pre-commit run

23c24f7

I have no problem, but codespell does

add slsqp for proximity

71928e1

add gfortran

6274e0b

fix variable shadowing

58a454c

fortran compiler for CI

73da3f9

first translation from f to c

8069a76

Initial file created with f2c (https://www.netlib.org/f2c/). Modified to be standalone (without f2c.h)

local variables aren't static

3a5d674

remove assigned format

5e553fc

j042 and others added 3 commits May 2, 2024 20:34

add macos-14 runners

978d697

add mac14 minimal and remove debug for windows

4de73bf

Merge pull request #424 from tataratat/ft-slsqp

fcdf1c4

slsqp for proximity

j042 added the cibuildwheel-test Ask ci to test all wheel builds label May 3, 2024

j042 changed the title ~~Levenberg-Marquart cool?~~ Levenberg-Marquart + SLSQP for Proximity May 3, 2024

add LICENSE for slsqp

d0bd269

j042 requested review from clemens-fricke and danielwolff1 May 3, 2024 10:26

j042 removed the request for review from clemens-fricke May 3, 2024 10:33

j042 added the enhancement New feature or request label May 3, 2024

danielwolff1 approved these changes May 3, 2024

View reviewed changes

j042 and others added 4 commits May 4, 2024 11:18

Apply suggestions from code review

85b0245

Co-authored-by: Daniel Wolff <[email protected]>

remove unused functions

3286901

rename parameter

8b6574c

parameter order suggestion from @danielwolff1

5923d0d

main return first, then additional ones. This applies to GuessMinusQuery, J, dJdu, d2Jdu2

j042 force-pushed the ft-LM-test branch from 90dc1d3 to 5923d0d Compare May 4, 2024 10:10

danielwolff1 approved these changes May 4, 2024

View reviewed changes

j042 added 3 commits May 6, 2024 09:26

update comment and compute status before exit

d094c58

add mode check for slsqp

14a174f

Queries outside of the spline sometimes does not return 0.0 convergence_norm. However, slsqp exits with 0 if J itself converges.

ensure retrun values

6199042

j042 force-pushed the ft-LM-test branch from 6d7b2b7 to 6199042 Compare May 6, 2024 12:46

set default to 1e-14

fe13a70

j042 added 2 commits May 6, 2024 16:32

rm outdated line

a95b8ea

add #progma once

b88a6bd

j042 merged commit 5e39cfb into main May 7, 2024
21 checks passed

j042 deleted the ft-LM-test branch May 7, 2024 13:37

markriegler pushed a commit that referenced this pull request Aug 6, 2024

finalize #394

902e5cf

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Levenberg-Marquart + SLSQP for Proximity #394

Levenberg-Marquart + SLSQP for Proximity #394

jzwar commented Mar 11, 2024 •

edited by j042

Loading

j042 commented Mar 11, 2024

j042 commented Apr 26, 2024

j042 commented Apr 26, 2024

j042 commented Apr 26, 2024

j042 commented Apr 26, 2024

j042 left a comment

j042 Apr 26, 2024

j042 Apr 26, 2024

j042 commented May 3, 2024

danielwolff1 left a comment

danielwolff1 commented May 4, 2024

j042 commented May 6, 2024

	lhs(i, i) = 2 * (spline_gradient_AAt(i, i) + lambda);
	lhs(i, i) = (2 * (spline_gradient_AAt(i, i)) + lambda;

Levenberg-Marquart + SLSQP for Proximity #394

Levenberg-Marquart + SLSQP for Proximity #394

Conversation

jzwar commented Mar 11, 2024 • edited by j042 Loading

Overview

Addressed Issues

Checklists

j042 commented Mar 11, 2024

j042 commented Apr 26, 2024

j042 commented Apr 26, 2024

j042 commented Apr 26, 2024

j042 commented Apr 26, 2024

j042 left a comment

Choose a reason for hiding this comment

j042 Apr 26, 2024

Choose a reason for hiding this comment

j042 Apr 26, 2024

Choose a reason for hiding this comment

j042 commented May 3, 2024

danielwolff1 left a comment

Choose a reason for hiding this comment

danielwolff1 commented May 4, 2024

j042 commented May 6, 2024

jzwar commented Mar 11, 2024 •

edited by j042

Loading