PERF: Fill jsj (JacobianOfSpatialJacobian) in-place and remove jsj1

[N-Dekker]

Used modern C++ range-based for loops to iterate over the matrices of jsj, in both GetJacobianOfSpatialJacobianUseComposition overloads. Reduced dynamic memory usage by removing jsj1 in those two cases.

Follow-up to pull request #890 commit b9ea3a8

---

This commit appears to make the registration part of itkElastixRegistrationMethodTest ~5% faster, going from ~33 seconds to 32-31 seconds on my PC. Timeouts of itkElastixRegistrationMethodTest at the CI of ITKElastix are a showstopper right now.

Update: I just tested the very same registration with elastix 5.1.0 (released last January) as well and it's ~34 seconds, so slightly slower than the current revision from main. Which is cool 😃

[N-Dekker]

This jsj.resize(nonZeroJacobianIndices.size()) may no longer be necessary, but it's hard for me decide, just by looking at those few lines of code.

[stefanklein]

Sounds good, have you tested it in a case of B-spline transform with affine transform as initial transform, (in which case the matrix is quite big, so it is a good test case)? Best, Stefan From: Niels Dekker ***@***.***> Sent: Thursday, May 11, 2023 11:42 AM To: SuperElastix/elastix ***@***.***> Cc: Stefan Klein ***@***.***>; Review requested ***@***.***> Subject: Re: [SuperElastix/elastix] PERF: Fill `jsj` (JacobianOfSpatialJacobian) in-place and remove `jsj1` (PR #891) 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. @N-Dekker<https://github.com/N-Dekker> requested your review on: #891<#891> PERF: Fill jsj (JacobianOfSpatialJacobian) in-place and remove jsj1. - Reply to this email directly, view it on GitHub<#891 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAF2LNOVUSXW2FUKSXWOMNTXFSX5DANCNFSM6AAAAAAX53WRCE>. You are receiving this because your review was requested.Message ID: ***@***.******@***.***>>

[N-Dekker]

Sounds good, have you tested it in a case of B-spline transform with affine transform as initial transform, (in which case the matrix is quite big, so it is a good test case)?

Thanks @stefanklein I tested with the "default" sequence of parameter maps from ElastixRegistrationMethod, translation + affine + bspline:

elastix/Core/Main/itkElastixRegistrationMethod.hxx

Lines 59 to 61 in b9ea3a8

	defaultParameterObject->AddParameterMap(elx::ParameterObject::GetDefaultParameterMap("translation"));
	defaultParameterObject->AddParameterMap(elx::ParameterObject::GetDefaultParameterMap("affine"));
	defaultParameterObject->AddParameterMap(elx::ParameterObject::GetDefaultParameterMap("bspline"));

(Those default parameter maps are created by ParameterObject::GetDefaultParameterMap)

So I think it's a good test case indeed 😇 Although it is quite time consuming, even with this pull request.

[stefanklein]

Ok that’s a great test! tnx From: Niels Dekker ***@***.***> Sent: Thursday, May 11, 2023 2:06 PM To: SuperElastix/elastix ***@***.***> Cc: Stefan Klein ***@***.***>; Mention ***@***.***> Subject: Re: [SuperElastix/elastix] PERF: Fill `jsj` (JacobianOfSpatialJacobian) in-place and remove `jsj1` (PR #891) 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. Sounds good, have you tested it in a case of B-spline transform with affine transform as initial transform, (in which case the matrix is quite big, so it is a good test case)? Thanks @stefanklein<https://github.com/stefanklein> I tested with the "default" sequence of parameter maps from ElastixRegistrationMethod, translation + affine + bspline: https://github.com/SuperElastix/elastix/blob/b9ea3a8e65a3e015a5a0f06acfb5bd22fad8dacc/Core/Main/itkElastixRegistrationMethod.hxx#L59-L61 (Those default parameter maps are created by ParameterObject::GetDefaultParameterMap<https://github.com/SuperElastix/elastix/blob/b9ea3a8e65a3e015a5a0f06acfb5bd22fad8dacc/Core/Main/elxParameterObject.cxx#L395>) So I think it's a good test case indeed 😇 Although it is quite time consuming, even with this pull request. — Reply to this email directly, view it on GitHub<#891 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAF2LNKS7SL4ELFJV6YJKVLXFTI2JANCNFSM6AAAAAAX53WRCE>. You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>