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

Reorganize SVD #1128

Merged
merged 3 commits into from
Nov 13, 2024
Merged

Reorganize SVD #1128

merged 3 commits into from
Nov 13, 2024

Conversation

aprokop
Copy link
Contributor

@aprokop aprokop commented Aug 2, 2024

Main motivation is to provide the SVD utility (without the inverse) to be used in OBB. Additionally, this patch removes the symmetricPseudoInverseSVD function that is not being used. Given that the new functionality is to be used in more than just interpolation, the file was renamed.

@aprokop aprokop added the refactoring Code reorganization label Aug 2, 2024
Copy link
Contributor

@dalg24 dalg24 left a comment

Choose a reason for hiding this comment

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

The PR is entitled "reorganize" but this is mostly a rename and change in the API of the kernels. I expected you were going to move the functionality, possibly to KokkosExt or whatnot.
Did you only want to rename the file?

src/interpolation/details/ArborX_DetailsSymmetricSVD.hpp Outdated Show resolved Hide resolved
@aprokop
Copy link
Contributor Author

aprokop commented Aug 6, 2024

Did you only want to rename the file?

This patch:

  • Renames the file
  • Gets rid of the unused batch kernel to run pseudo-inverse SVD on multiple matrices
  • Moves out computing SVD out of the computing pseudo-inverse SVD so that one can now get the SVD result back

I expected you were going to move the functionality, possibly to KokkosExt or whatnot.

I consider it moved out of the interpolation to just Details. I have not considered KokkosExt, though one can argue it could belong to KokkosKernels.

@aprokop aprokop merged commit 1c9ff1a into arborx:master Nov 13, 2024
1 of 2 checks passed
@aprokop aprokop deleted the sym_svd branch November 13, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Code reorganization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants