Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

COMP: Remove Fortran and LAPACK #294

Merged
merged 1 commit into from
Feb 8, 2023
Merged

Conversation

allemangD
Copy link
Collaborator

Update SPHARM-PDM and GROUPS to remove dependency on Fortran and LAPACK.

https://github.com/slicersalt/SPHARM-PDM/commits/slicersalt-2023-02-06-dfde14b72

https://github.com/slicersalt/GROUPS/commits/slicersalt-2023-02-06-037ab7dcd

Note that the SPHARM-PDM fork and SHA should be updated after NIRALUser/GROUPS#44 is merged.


Compared results on the hippocampus sample data; maximum absolute distance between correspondent points is 0.00075 (total length is 92.2; something like 1e-6 relative tolerance). Computed via model-to-model distance module.

I will continue to test on more data and post results here; but I am confident that there are no glaring errors in the Eigen translation.

Copy link
Contributor

@bpaniagua bpaniagua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@allemangD
Copy link
Collaborator Author

I ran SPHARM on the correspondence improvement dataset; the tolerance is not so good on the hourglass example.

image

You can see the maximum absolute deflection from the lapack output is 0.65; a relative tolerance of about 5e-3.

Here's an axis aligned anterior view.

image

I'll try switching the solver which SPHARM uses to HouseholderQR as mentioned in NIRALUser/SPHARM-PDM#81 to see if this improves the tolerance.

Visually, the eigen output looks the same as the lapack output.

@allemangD
Copy link
Collaborator Author

allemangD commented Feb 7, 2023

Tolerance is slightly better with HouseholderQR, but not significantly (0.56 maximum deflection vs 0.65).

image

@allemangD
Copy link
Collaborator Author

allemangD commented Feb 8, 2023

I ran the test on the entireity of the tricuspid valve leaflets dataset; maximum point-to-point deflection across the whole dataset is 0.014, but mean is 0.000 (MeshStatistics doesn't output more than 3 decimals).

Worth noting that outlier is a single point in a single case. The mean and stddev across the dataset are both 0.000.

image


Overall I'm confident switching to Eigen doesn't introduce any regressions. I believe the next effort should be integrating the scripts I developed to do these deflection analyses into integration tests for the SPHARM-PDM module, so that as refactoring continues we can be sure no regressions are introduced.


I will merge NIRALUser/GROUPS#44 and NIRALUser/SPHARM-PDM#81, update git tags, and finally merge this PR.

@allemangD allemangD merged commit 70b6e58 into Kitware:master Feb 8, 2023
@jcfr jcfr linked an issue Feb 21, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove LAPACK as a requirement
2 participants