search

JuliaManifolds / Manopt.jl

🏔️Manopt. jl – Optimization on Manifolds in Julia
http://manoptjl.org
Other
314 stars 40 forks source link

check_Hessian: retraction_method is not passed to gradient check #342

Closed rs1909 closed 8 months ago

rs1909 commented 8 months ago

I believe that retraction_method keyword parameter is not passed onto the check_gradient call within check_Hessian:

https://github.com/JuliaManifolds/Manopt.jl/blame/a24c869897c732b510c71fcc2d7693f819e925d1/src/helpers/checks.jl#L297

Calling check_gradient directly does work with the same retraction, while check_Hessian fails.

kellertuer commented 8 months ago

Oh, well spotted :) Thanks for opening this issue, we must have missed that.

Do you want to open an PR? Otherwise I could do that later today (in Central Europe Time than is)

rs1909 commented 8 months ago

Many thanks. Sorry I'd prefer not to create a PR, as I have never done that. It is not urgent either, I've just switched off gradient checking and done it separately, which works well.

On Sat, Jan 6, 2024 at 6:34 AM Ronny Bergmann @.***> wrote:

Oh, well spotted :) Thanks for opening this issue, we must have missed that.

Do you want to open an PR? Otherwise I could do that later today (in Central Europe Time than is)

— Reply to this email directly, view it on GitHub https://github.com/JuliaManifolds/Manopt.jl/issues/342#issuecomment-1879573130, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQIFI7F65FZ36OVJ6TB7V3YNDWA5AVCNFSM6AAAAABBPFAD72VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZZGU3TGMJTGA . You are receiving this because you authored the thread.Message ID: @.***>

kellertuer commented 8 months ago

No problem, I will do it later today then.

Since you re running it separately, do you think we missed any other keyword?

kellertuer commented 8 months ago

As you can see I just did the pull request. Later today, when all tests have passed I will merge it and register a new version, so probably tomorrow morning you will have Manopt.jl 0.4.47 where the bug is fixed.

Thanks again for reporting this.