TFactor: bulkerify GEMV (both MC and GPU) #1214

albestro · 2024-11-15T17:50:06Z

In this PR we try to increase parallelisation of GEMV step of TFactor, which is by construction serial (all results goes into the same block tile), by using workspaces for storing partial results and then reducing them before the final TRMV step.

Algorithmically, the main change is that stepGEMV loop has been replaced with a single stepGEMVAll, and the concept has been applied in a similar way for both backends, MC and GPU:

MC: pika::bulk splits input tiles over multiple task, each one stores the partial results in their workspace and just one at the end does the reduction.
GPU: similarly to the CPU, the work is forked over different pika tasks, each one computing a partial result on a different GPU stream. These tasks then join into a single task which performs the reduction.

In order to implement this solution, workspaces for intermediate results have been added (there is another option which does not require any additional workspace nor reduction, and that we might explore for MC in another PR).

TODO:

This work is based on TFactor: simplification and cleanup #1213
check with @msimberg if we can do something different instead of the "fork" (e.g. a bulk for the GPU but each pika thread should get a different cuda stream)
define better how many workspaces are needed (currently a random choice)
profile and test performance of this solution
Verify if there is anything to be "ported" from computeTFactor parallelise computation with bulk (MC Local and Distributed) #798
workers default value should be backend dependent

Close #798.

EDIT: since this is conceptually similar to #798 and it is going to be closed as soon as this gets merged, I migrated here the doc fixes happened there.

albestro · 2024-12-02T16:56:41Z

cscs-ci run

just to check if there is any major problems

rasolca

LGTM.
Please name new functions in snake case, and rename existing internal functions if possible.

rasolca · 2024-12-04T10:58:46Z

include/dlaf/factorization/qr/t_factor_impl.h

+    matrix::Matrix<T, Device::GPU> ws_T({nworkspaces * nrefls_step, nrefls_step},
+                                        {nrefls_step, nrefls_step});


All Ts are allocated at scheduling. Better reuse it.

msimberg

Only a few questions, nothing blocking.

include/dlaf/eigensolver/bt_reduction_to_band/impl.h

include/dlaf/factorization/qr/api.h

include/dlaf/factorization/qr/t_factor_impl.h

rasolca · 2024-12-06T10:58:27Z

include/dlaf/factorization/qr/internal/get_tfactor_nworkers.h

+
+namespace dlaf::factorization::internal {
+
+inline size_t getTFactorNWorkers() noexcept {


What about this to be snake case?

see 36f179d

include/dlaf/tune.h

albestro · 2024-12-10T14:37:50Z

cscs-ci run

albestro · 2024-12-11T16:16:27Z

Converted to draft in order to prevent merging, since I'm still doing some checks.

include/dlaf/tune.h

taus are set for gemv, but then full tile is reduced, hence also the diag with taus.

tile_t might get moved by first worker before any other work is able to use its size

in case of odd division of tiles, it might end up being selected but not used.

albestro · 2024-12-17T13:53:01Z

cscs-ci run

codecov-commenter · 2024-12-17T14:47:32Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 91.76471% with 14 lines in your changes missing coverage. Please review.

Project coverage is 94.29%. Comparing base (b82a100) to head (d199b88).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
include/dlaf/factorization/qr/t_factor_impl.h	88.59%	10 Missing and 3 partials ⚠️
src/tune.cpp	0.00%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1214      +/-   ##
==========================================
- Coverage   94.38%   94.29%   -0.10%     
==========================================
  Files         154      155       +1     
  Lines        9374     9469      +95     
  Branches     1160     1169       +9     
==========================================
+ Hits         8848     8929      +81     
- Misses        320      331      +11     
- Partials      206      209       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

add assert also for gpu code that calls add manually

albestro · 2025-01-07T15:52:42Z

cscs-ci run

albestro · 2025-01-07T17:02:12Z

Apart the fixes committed between the two last CI runs, which fix for sure some problems, one major difference between the CI runs is that the oldest one ran on Eiger while the latest ran on Daint (because of the rebase on master).

The oldest one on Eiger had some failures that are concerning, specifically all the CPU UNIT_RANK_6, which failed due to a timeout on Eigensolver/GenEigensolver MC Distributed test (example)

I will try to investigate why that's the case and why it haven't happened on Daint.

albestro · 2025-01-20T10:37:33Z

include/dlaf/factorization/qr/internal/get_tfactor_nworkers.h

+
+inline size_t get_tfactor_nworkers() noexcept {
+  // Note: precautionarily we leave at least 1 thread "free" to do other stuff (if possible)
+  const std::size_t available_workers = pika::resource::get_thread_pool("default").get_os_thread_count();


this does not make sense, since the worker is backend dependent (i.e. on MC a worker is a thread, on a GPU is a stream).

albestro added the Type:Optimization label Nov 15, 2024

albestro added this to the Optimizations milestone Nov 15, 2024

albestro self-assigned this Nov 15, 2024

albestro force-pushed the alby/tfactor-optim/bulk branch from 6558780 to bf17fcc Compare November 18, 2024 13:16

Base automatically changed from alby/tfactor-optim/no-gemv-divergence to master November 22, 2024 11:28

albestro force-pushed the alby/tfactor-optim/bulk branch 4 times, most recently from 37ea75b to 2845339 Compare December 2, 2024 16:48

albestro requested review from msimberg, rasolca and RMeli December 3, 2024 16:08

albestro marked this pull request as ready for review December 3, 2024 16:08

rasolca requested changes Dec 4, 2024

View reviewed changes

albestro requested a review from rasolca December 5, 2024 11:22

msimberg approved these changes Dec 5, 2024

View reviewed changes

include/dlaf/eigensolver/bt_reduction_to_band/impl.h Outdated Show resolved Hide resolved

include/dlaf/factorization/qr/api.h Outdated Show resolved Hide resolved

include/dlaf/factorization/qr/t_factor_impl.h Outdated Show resolved Hide resolved

albestro force-pushed the alby/tfactor-optim/bulk branch from 05dc48d to f83f317 Compare December 5, 2024 15:00

rasolca reviewed Dec 6, 2024

View reviewed changes

msimberg reviewed Dec 9, 2024

View reviewed changes

include/dlaf/tune.h Outdated Show resolved Hide resolved

albestro force-pushed the alby/tfactor-optim/bulk branch from 0397c9e to d370894 Compare December 9, 2024 09:47

albestro requested a review from rasolca December 9, 2024 09:47

rasolca approved these changes Dec 10, 2024

View reviewed changes

msimberg approved these changes Dec 10, 2024

View reviewed changes

albestro marked this pull request as draft December 11, 2024 16:15

albestro commented Dec 11, 2024

View reviewed changes

include/dlaf/tune.h Show resolved Hide resolved

albestro added 2 commits December 13, 2024 14:21

factor out gemvLoop for MC

b1197be

mc-local: add stepGEMVAll with bulk using workspaces

9647ec8

albestro added 10 commits December 13, 2024 14:21

bug fix: tfactor workspaces allocation

87c952b

bug fix: replace sender workspace instead of appending

3d3b93c

bug fix: missing default initialization to a proper value

92aa5b7

bug fix: diag has to be reset at the end

eb9def0

taus are set for gemv, but then full tile is reduced, hence also the diag with taus.

bug fix: not all workspaces should be used

202f1cc

bug fix: race-condition on k using tile_t

c0a7660

tile_t might get moved by first worker before any other work is able to use its size

avoid moves by using const& in loop_gemv

dec050a

minor changes

fc12aae

minor change: drop unused step_trmv and reorder gpu helpers

3f2a53a

minor changes and cleanup to doc

5997e52

albestro force-pushed the alby/tfactor-optim/bulk branch from f0638e1 to 5997e52 Compare December 17, 2024 09:10

bug fix: worker might not get used anyway

383dde5

in case of odd division of tiles, it might end up being selected but not used.

albestro added 2 commits December 19, 2024 16:41

bug fix: workspace for reduction to band was too big.

5f2ddd4

add assert also for gpu code that calls add manually

bug fix: workspace in bt_red2band migth have not been reset

ffd3135

albestro mentioned this pull request Dec 20, 2024

benchmark scripts: basic separation for alps vcluster, number of cores, gpu defaults, ... #1254

Draft

1 task

fix conflict

d199b88

albestro force-pushed the alby/tfactor-optim/bulk branch from 2f90f23 to d199b88 Compare December 20, 2024 15:14

albestro mentioned this pull request Jan 8, 2025

TFactor: use optimised kernel #1256

Draft

2 tasks

albestro commented Jan 20, 2025

View reviewed changes

albestro added 4 commits January 21, 2025 17:38

fix number of workers

32e9469

minor: rename worker_id to make it homogeneous among different backends

251e405

reshape each tile workspace

29047c9

minor doc fix

a22f5a0

albestro force-pushed the alby/tfactor-optim/bulk branch from bc153ce to 152f8e7 Compare January 21, 2025 16:40

add check for equal size for workspace and tile_t, and related doc

0d2bd4f

albestro force-pushed the alby/tfactor-optim/bulk branch from 152f8e7 to 0d2bd4f Compare January 21, 2025 16:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TFactor: bulkerify GEMV (both MC and GPU) #1214

TFactor: bulkerify GEMV (both MC and GPU) #1214

albestro commented Nov 15, 2024 •

edited

Loading

albestro commented Dec 2, 2024

rasolca left a comment

rasolca Dec 4, 2024

msimberg left a comment

rasolca Dec 6, 2024

albestro Dec 6, 2024

albestro commented Dec 10, 2024

albestro commented Dec 11, 2024

albestro commented Dec 17, 2024

codecov-commenter commented Dec 17, 2024 •

edited

Loading

albestro commented Jan 7, 2025

albestro commented Jan 7, 2025

albestro Jan 20, 2025

		matrix::Matrix<T, Device::GPU> ws_T({nworkspaces * nrefls_step, nrefls_step},
		{nrefls_step, nrefls_step});


		namespace dlaf::factorization::internal {

		inline size_t getTFactorNWorkers() noexcept {

TFactor: bulkerify GEMV (both MC and GPU) #1214

Are you sure you want to change the base?

TFactor: bulkerify GEMV (both MC and GPU) #1214

Conversation

albestro commented Nov 15, 2024 • edited Loading

albestro commented Dec 2, 2024

rasolca left a comment

Choose a reason for hiding this comment

rasolca Dec 4, 2024

Choose a reason for hiding this comment

msimberg left a comment

Choose a reason for hiding this comment

rasolca Dec 6, 2024

Choose a reason for hiding this comment

albestro Dec 6, 2024

Choose a reason for hiding this comment

albestro commented Dec 10, 2024

albestro commented Dec 11, 2024

albestro commented Dec 17, 2024

codecov-commenter commented Dec 17, 2024 • edited Loading

Codecov Report

albestro commented Jan 7, 2025

albestro commented Jan 7, 2025

albestro Jan 20, 2025

Choose a reason for hiding this comment

albestro commented Nov 15, 2024 •

edited

Loading

codecov-commenter commented Dec 17, 2024 •

edited

Loading