JuliaImageRecon / RegularizedLeastSquares.jl

MIT License
20 stars 9 forks source link

[Bug] ADMM verbose doesn't work anymore #93

Closed SebastianFlassbeck closed 3 weeks ago

SebastianFlassbeck commented 1 month ago

If ADMM is run with verbose = true, it throws an error in line 207 in ADMM.jl since the variable iteration doesn't exist. I'm guessing that state.iteration instead, should fix it but I didn't look through the code thoroughly.

nHackel commented 1 month ago

Yes that should fix it. I can push that change to master but the earliest time I can do that is in 2+ days

nHackel commented 1 month ago

Maybe a more fundamental question is if we can replace verbose with some callback that has verbose printing for each solver or if we can replace verbose with Julias logging system.

I havent used the keyword yet, so Im not sure whats the most convenient from a user perspective. Do you have a preference?

Or maybe @JakobAsslaender you have one?

I could maybe sketch up verbosecallback in the next week

JakobAsslaender commented 1 month ago

Hmm, my first instinct is that a callback is a rather complicated way of printing some convergence measures. Beyond this first instinct, I would try and keep it similar to other packages, but I would need look into the common practices.

SebastianFlassbeck commented 1 month ago

Thanks. I don't have any preferences. FYI, the outputs in lines 267-271 also need a state..

nHackel commented 3 weeks ago

Should be fixed with the latest (currently being released) version. I've also added tests for the verbose keyword