PERF: Speed up ComputeJacobianTerms access to vnl_sparse_matrix
#959
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added `getCovariance` lambda as a faster alternative to `vnl_sparse_matrix::operator()(unsigned int, unsigned int)`. Replaced `cov(r, c)` calls with `getCovariance(r, c)`. `getCovariance` offers constant-time access to the back of each row. It adds a columns to the end of the row without having to iterate over the entire row. Moreover, when iteration over the entire row is really necessary, it has a simpler loop condition, compared to `vnl_sparse_matrix::operator()`: `(it->first < c)` rather than `(ri != rw.end()) && ((*ri).first < c)`. Comparison based on the `vnl_sparse_matrix::operator()` implementation at https://github.com/InsightSoftwareConsortium/ITK/blob/36d2c5ac368cf7917d8e73806c6eaa1f195b85c1/Modules/ThirdParty/VNL/src/vxl/core/vnl/vnl_sparse_matrix.hxx#L411-L426 Related pull request: vxl/vxl#926 "PERF: Speed up `vnl_sparse_matrix::operator()`"
|
The CI failures seem unrelated to the code changes:
|
N-Dekker
added a commit
to N-Dekker/ITKElastix
that referenced
this pull request
Sep 15, 2023
Supports passing a transform retrieved by `GetCombinationTransform()` or `GetNthTransform(n)` to `ConvertToItkTransform`, when an _external_ initial transform was specified before running the registration. Also including various performance improvements. Including: pull request SuperElastix/elastix#946 commit SuperElastix/elastix@7bcb2b3 ENH: Write external initial transforms to ITK transform file pull request SuperElastix/elastix#949 commit SuperElastix/elastix@601f465 ENH: `ConvertToItkTransform` should support external initial transform pull request SuperElastix/elastix#950 commit SuperElastix/elastix@7e883bf ENH: Add GetInitialTransform() and GetExternalInitialTransform() pull request SuperElastix/elastix#959 commit SuperElastix/elastix@3979978 PERF: Speed up ComputeJacobianTerms access to `vnl_sparse_matrix` pull request SuperElastix/elastix#960 commit SuperElastix/elastix@4d028f7 PERF: Speed up full, grid, sparse mask samplers by a local sampleVector
N-Dekker
added a commit
to N-Dekker/ITKElastix
that referenced
this pull request
Sep 18, 2023
Supports passing a transform retrieved by `GetCombinationTransform()` or `GetNthTransform(n)` to `ConvertToItkTransform`, when an _external_ initial transform was specified before running the registration. Also including various performance improvements. Including: pull request SuperElastix/elastix#946 commit SuperElastix/elastix@7bcb2b3 ENH: Write external initial transforms to ITK transform file pull request SuperElastix/elastix#949 commit SuperElastix/elastix@601f465 ENH: `ConvertToItkTransform` should support external initial transform pull request SuperElastix/elastix#950 commit SuperElastix/elastix@7e883bf ENH: Add GetInitialTransform() and GetExternalInitialTransform() pull request SuperElastix/elastix#959 commit SuperElastix/elastix@3979978 PERF: Speed up ComputeJacobianTerms access to `vnl_sparse_matrix` pull request SuperElastix/elastix#960 commit SuperElastix/elastix@4d028f7 PERF: Speed up full, grid, sparse mask samplers by a local sampleVector
dzenanz
pushed a commit
to InsightSoftwareConsortium/ITKElastix
that referenced
this pull request
Sep 19, 2023
Supports passing a transform retrieved by `GetCombinationTransform()` or `GetNthTransform(n)` to `ConvertToItkTransform`, when an _external_ initial transform was specified before running the registration. Also including various performance improvements. Including: pull request SuperElastix/elastix#946 commit SuperElastix/elastix@7bcb2b3 ENH: Write external initial transforms to ITK transform file pull request SuperElastix/elastix#949 commit SuperElastix/elastix@601f465 ENH: `ConvertToItkTransform` should support external initial transform pull request SuperElastix/elastix#950 commit SuperElastix/elastix@7e883bf ENH: Add GetInitialTransform() and GetExternalInitialTransform() pull request SuperElastix/elastix#959 commit SuperElastix/elastix@3979978 PERF: Speed up ComputeJacobianTerms access to `vnl_sparse_matrix` pull request SuperElastix/elastix#960 commit SuperElastix/elastix@4d028f7 PERF: Speed up full, grid, sparse mask samplers by a local sampleVector
N-Dekker
added a commit
that referenced
this pull request
Jul 2, 2024
- Now that pull request vxl/vxl#926 commit vxl/vxl@653919f "PERF: Speed up `vnl_sparse_matrix::operator()`" is merged, it appears no longer useful to maintain our own `getCovariance` alternative. This reverts pull request #959 commit 3979978 "PERF: Speed up ComputeJacobianTerms access to `vnl_sparse_matrix`"
N-Dekker
added a commit
that referenced
this pull request
Jul 3, 2024
- Now that pull request vxl/vxl#926 commit vxl/vxl@653919f "PERF: Speed up `vnl_sparse_matrix::operator()`" is merged, it appears no longer useful to maintain our own `getCovariance` alternative. This reverts pull request #959 commit 3979978 "PERF: Speed up ComputeJacobianTerms access to `vnl_sparse_matrix`"
N-Dekker
added a commit
that referenced
this pull request
Jul 3, 2024
- Now that pull request vxl/vxl#926 commit vxl/vxl@653919f "PERF: Speed up `vnl_sparse_matrix::operator()`" is merged, it appears no longer useful to maintain our own `getCovariance` alternative. This reverts pull request #959 commit 3979978 "PERF: Speed up ComputeJacobianTerms access to `vnl_sparse_matrix`"
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added
getCovariancelambda as a faster alternative tovnl_sparse_matrix::operator()(unsigned int, unsigned int). Replacedcov(r, c)calls withgetCovariance(r, c).getCovarianceoffers constant-time access to the back of each row. It adds a columns to the end of the row without having to iterate over the entire row. Moreover, when iteration over the entire row is really necessary, it has a simpler loop condition, compared tovnl_sparse_matrix::operator():(it->first < c)rather than(ri != rw.end()) && ((*ri).first < c).Comparison based on the
vnl_sparse_matrix::operator()implementation at https://github.com/InsightSoftwareConsortium/ITK/blob/36d2c5ac368cf7917d8e73806c6eaa1f195b85c1/Modules/ThirdParty/VNL/src/vxl/core/vnl/vnl_sparse_matrix.hxx#L411-L426Related pull request: vxl/vxl#926 "PERF: Speed up
vnl_sparse_matrix::operator()"