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

TridiagSolver (local): embed row permutation in rank1 solver #936

Merged
merged 6 commits into from
Jul 19, 2023

Conversation

albestro
Copy link
Collaborator

Distributed and local implementations have slightly diverged in the last months.

This PR addresses the main (and probably the only ones) differences. With #831 distributed implementation got row permutations and setUnitDiag embedded in rank1 solution, removing the need of explicitly doing so just before doing the gemm. Local implementation was still doing the permutation separately.

With this PR both setUnitDiag and row permutation happen during rank1 solver also for the local implementation.

@albestro albestro requested a review from rasolca July 14, 2023 12:48
@albestro albestro self-assigned this Jul 14, 2023
@albestro
Copy link
Collaborator Author

Note:

Permutation is done in a slightly different way due to different optimization requirements between local and distributed. In the latter one, in order to avoid additional (and unnecessary) communications, the permutation is done in the first steps of the rank1-solver, and all other steps have to take this into account. On the other hand, in the local implementation the permutation is done in the last step, since there is no reason to do it immediately, keeping rank1-solver "clean" from permutations till the very end.

This also introduces a difference for k == 2, for which the permutations should happen even if it is handled differently from other cases. Since in local implementation is done at the very end, the aforementioned case has to handled differently by performing an ad-hoc permutation just before the early exit of the special case. It must be highlighted that these special cases are not tested extensively for the moment, and I'm going to open an issue to keep track of this.

@albestro albestro force-pushed the alby/rank1-local-permutation branch from cec3b83 to e728046 Compare July 14, 2023 13:56
@albestro albestro added this to the Cleanup milestone Jul 17, 2023
@rasolca
Copy link
Collaborator

rasolca commented Jul 17, 2023

cscs-ci run

@codecov-commenter
Copy link

codecov-commenter commented Jul 17, 2023

Codecov Report

Merging #936 (e728046) into master (3d86f2b) will decrease coverage by 0.01%.
The diff coverage is 96.42%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master     #936      +/-   ##
==========================================
- Coverage   93.20%   93.19%   -0.01%     
==========================================
  Files         136      136              
  Lines        8327     8318       -9     
  Branches     1081     1083       +2     
==========================================
- Hits         7761     7752       -9     
  Misses        391      391              
  Partials      175      175              
Impacted Files Coverage Δ
include/dlaf/eigensolver/tridiag_solver/kernels.h 100.00% <ø> (ø)
src/eigensolver/tridiag_solver/kernels.cpp 100.00% <ø> (ø)
src/eigensolver/tridiag_solver/kernels.cu 41.48% <ø> (-6.66%) ⬇️
include/dlaf/eigensolver/tridiag_solver/merge.h 99.81% <96.42%> (-0.19%) ⬇️

@albestro
Copy link
Collaborator Author

cscs-ci run

@albestro albestro force-pushed the alby/rank1-local-permutation branch from 768a2b1 to 677b346 Compare July 17, 2023 13:27
@albestro
Copy link
Collaborator Author

cscs-ci run

@rasolca rasolca merged commit 673e127 into master Jul 19, 2023
@rasolca rasolca deleted the alby/rank1-local-permutation branch July 19, 2023 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants