TriSolver (dist): move sorting permutation from CPU to GPU #1118
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
|
cscs-ci run |
|
cscs-ci run |
2 similar comments
|
cscs-ci run |
|
cscs-ci run |
albestro
force-pushed
the
alby/permute-local
branch
from
180c08d
to
217cf55
Compare
|
cscs-ci run |
2 similar comments
|
cscs-ci run |
|
cscs-ci run |
albestro
commented
May 27, 2024
| // @param perm_sorted array[n] current -> initial (i.e. evals[i] -> types[perm_sorted[i]]) | ||
| // @param index_sorted array[n] global(sort(non-deflated)|sort(deflated))) -> initial | ||
| // @param index_sorted_coltype array[n] local(sort(upper)|sort(dense)|sort(lower)|sort(deflated))) -> initial | ||
| // @param i5_lc array[n_lc] local(sort(upper)|sort(dense)|sort(lower)|sort(deflated))) -> initial |
There was a problem hiding this comment.
note-to-self: specify that they are local indices, while in index_sorted_coltype they are global indices
|
cscs-ci run |
rasolca
approved these changes
May 28, 2024
Comment on lines
55
to
59
| // Note: | ||
| // These are not implementation constraints, but more logic constraints. Indeed, these ensure that | ||
| // the range [i_begin, i_end] is square in terms of elements (it would not make sense to have it square | ||
| // in terms of number of tiles). Moreover, by requiring mat_in and mat_out matrices to have the same | ||
| // shape, it is ensured that range [i_begin, i_end] is actually the same on both sides. |
There was a problem hiding this comment.
Constraints should be revised (in a different PR).
| std::move(setup_permute_fn)) | | ||
| ex::unpack() | ex::bulk(subm_dist.size().get<C>(), permute_fn)); | ||
| ex::start_detached(std::move(sender) | ex::transfer(di::getBackendScheduler<Backend::MC>()) | | ||
| ex::bulk(nperms, std::move(permute_fn))); |
There was a problem hiding this comment.
Number of tasks created by bulk might be huge.
I suggest addressing in a new PR.
There was a problem hiding this comment.
The number might be large, but not larger than num_threads. That may of course still be too much, but just keep in mind that the thread_pool_scheduler specialization of bulk will not blindly create nperms tasks.
rasolca
added
Type:New Feature
|
cscs-ci run |
msimberg
approved these changes
May 28, 2024
|
Back to draft due to frequent hangs on santis |
with a distributed matrix
required to be compatible with both local and distributed usage
albestro
force-pushed
the
alby/permute-local
branch
from
11f2185
to
dd8c4ec
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in 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.
This PR aims at dropping the custom
permuteJustLocaland reduce the use-case, by transforming permutation indices, to be manageable with the existing local permutation implementation, that exists for both backends.It might be possible to drop i5 (for distributed implementation)permuteAPI? Should we separate the "distributed" use case (at least formally) or is it enough reviewing assumptions?Evaluate if it is worth switching toMatrixRef(just for the code changed)Notes
From PR #967 each rank sort eigenvalues by type (upper, dense, lower, deflated) independently from other ranks. At the time of that PR, for convenience reasons, we opted for performing the sort with a custom permutation procedure
permuteJustLocalthat were able to deal with global indices but just apply the permutation to the local part. In addition to this,permuteJustLocalwas implemented just on CPU because on GPU it would had required a major effort not worth due to the inherently GPU inefficient type of operations.