search

SuperElastix / elastix

Official elastix repository
http://elastix.dev
Apache License 2.0
449 stars 110 forks source link

BUG: Let `elx::ConjugateGradientFRPR` override ITK's FRPROptimizer #1118

Closed N-Dekker closed 1 month ago

N-Dekker commented 1 month ago

elx::ConjugateGradientFRPR member functions GetValueAndDerivative and LineOptimize did originally override the corresponding virtual member functions of ITK's FRPROptimizer. This property was broken by a change in the signature of those ITK member functions: ITK commit https://github.com/InsightSoftwareConsortium/ITK/commit/a088a95fca4e0f94ef661cdc43a6bec26ad687a9, "PERF: Changed to pass by reference for faster performance", 2008-04-23

This commit restores those "overrides", by adjusting the signature of the elx::ConjugateGradientFRPR member functions, in accordance with those ITK member functions.

Bug found by macos-12/clang compiler warnings, like:

warning: 'elastix::ConjugateGradientFRPR::GetValueAndDerivative' hides overloaded virtual function [-Woverloaded-virtual]

Note: These two elx::ConjugateGradientFRPR member functions don't seem to be tested at the CI, currently.

N-Dekker commented 1 month ago

While it may or may not be an important bug fix, I think it must be merged anyway!

stefanklein commented 1 month ago

Agree!

From: Niels Dekker @.> Sent: Tuesday, May 14, 2024 5:12 PM To: SuperElastix/elastix @.> Cc: Stefan Klein @.>; Mention @.> Subject: Re: [SuperElastix/elastix] BUG: Let elx::ConjugateGradientFRPR override ITK's FRPROptimizer (PR #1118)

Waarschuwing: Deze e-mail is afkomstig van buiten de organisatie. Klik niet op links en open geen bijlagen, tenzij u de afzender herkent en weet dat de inhoud veilig is. Caution: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.

While it may or may not be an important bug fix, I think it must be merged anyway!

- Reply to this email directly, view it on GitHubhttps://github.com/SuperElastix/elastix/pull/1118#issuecomment-2110498189, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAF2LNLK4CD3OGB6KDFVMN3ZCISVPAVCNFSM6AAAAABHWJYO5GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJQGQ4TQMJYHE. You are receiving this because you were mentioned.Message ID: @.**@.>>