Move functor-level GEMM routines to KokkosBlas #1519

mzuzek · 2022-09-08T11:56:19Z

This PR moves GEMM

Status:

Implementation:
- move implementations of SerialGemm, TeamGemm and TeamVectorGemm from KokkosBatched to KokkosBlas (BatchedSerialGemm stays in batched);
- move MKL implementation to dedicated header;
Interface:
- move selective Gemm interface (dispatching between SerialGemm and TeamGemm) from KokkosBatched to KokkosBlas;
Tests:
- move unit tests for functor-level routines;

fnrizzi · 2022-09-08T12:00:58Z

just my opnion, but i think it would make sense to keep this PR only about moving things around, and then address the implementation of ConjTranspose and scalar stuff in a separate PR

kokkos-devops-admin · 2022-09-08T12:08:12Z

Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
NO INSPECTION HAS BEEN PERFORMED ON THIS PULL REQUEST! - This PR must be inspected by setting label 'AT: PRE-TEST INSPECTED'.

mzuzek · 2022-09-08T13:06:09Z

@lucbv @fnrizzi

Here are functor-level routines in their respective header files and namespaces - please let me know if you wish any adjustments in naming of file structure:

src

edit 2022-09-26: moved impl to top header, #1504 file structure changes, added MKL, added implementations to chart (blue boxes)

mzuzek · 2022-09-09T15:32:46Z

src/blas/KokkosBlas3_gemm_inner_fix.hpp

+
+#include "KokkosBlas3_serial_gemm_inner_fixa_impl.hpp"
+// TODO: fix compilation errors InnerGemmFixB (not used internally, not tested)
+// #include "KokkosBlas3_serial_gemm_inner_fixb_impl.hpp"


Compiler errors in this header were not detected before, because it was not used anywhere (no internal need for InnerGemmFixB and no tests for it).

mzuzek · 2022-09-09T15:34:31Z

src/blas/KokkosBlas3_gemm.hpp

+///
+template <typename ArgTransA, typename ArgTransB, typename ArgMode,
+          typename ArgAlgo = Algo::Gemm::Default>
+struct Gemm {


@lucbv Should this interface be placed in KokkosBlas::Experimental namespace ? (it wasn't in KokkosBatched)

No need for experimental namespace here

mzuzek · 2022-09-09T15:35:17Z

@lucbv @fnrizzi
I reviewed the headers and interfaces trying to apply what we've discussed, so let me open the PR for reviews.

For the following tasks, I'll create follow-up PRs:

implement ConjTranspose (follow-up: cover in tests);
implement mixed input scalar types (value types in A, B and C matrices);
extend unit tests to cover Algo::Gemm::CompactMKL (MKL implementation that works on SIMD vectors);

kokkos-devops-admin · 2022-09-09T16:31:10Z

Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
NO INSPECTION HAS BEEN PERFORMED ON THIS PULL REQUEST! - This PR must be inspected by setting label 'AT: PRE-TEST INSPECTED'.

kokkos-devops-admin · 2022-09-15T07:37:04Z

Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
NO INSPECTION HAS BEEN PERFORMED ON THIS PULL REQUEST! - This PR must be inspected by setting label 'AT: PRE-TEST INSPECTED'.

kokkos-devops-admin · 2022-09-20T23:52:58Z

Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
NO INSPECTION HAS BEEN PERFORMED ON THIS PULL REQUEST! - This PR must be inspected by setting label 'AT: PRE-TEST INSPECTED'.

kokkos-devops-admin · 2022-09-22T13:16:33Z

Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
NO INSPECTION HAS BEEN PERFORMED ON THIS PULL REQUEST! - This PR must be inspected by setting label 'AT: PRE-TEST INSPECTED'.

lucbv

You are defining a macro for COMPACT_MKL multiple times but it should really be centralized instead.

src/batched/dense/KokkosBatched_Gemm_Decl.hpp

lucbv · 2022-09-09T23:22:27Z

src/blas/KokkosBlas3_gemm.hpp

+///
+template <typename ArgTransA, typename ArgTransB, typename ArgMode,
+          typename ArgAlgo = Algo::Gemm::Default>
+struct Gemm {


No need for experimental namespace here

lucbv · 2022-09-09T23:23:17Z

src/blas/KokkosBlas3_gemm_inner_fix.hpp

+
+#include "KokkosBlas3_serial_gemm_inner_fixa_impl.hpp"
+// TODO: fix compilation errors InnerGemmFixB (not used internally, not tested)
+// #include "KokkosBlas3_serial_gemm_inner_fixb_impl.hpp"


mzuzek · 2022-09-22T19:49:26Z

@lucbv

You are defining a macro for COMPACT_MKL multiple times but it should really be centralized instead.

Thank you for pointing this out. On 34c50ea I've created common/impl/KokkosKernels_MKLUtils.hpp for common MKL utils used in blas and sparse (also fixed common/src/impl header path in CMake) and moved the duplicated macro there (also #inlcude <mkl.h> and KOKKOSKERNELS_MKL_SAFE_CALL). Let me know in case you'd wish a different location, name or content.

mzuzek · 2022-09-27T12:35:26Z

Added one trivial fix on 53c58bc and rebased on develop.

kokkos-devops-admin · 2022-09-27T12:36:21Z

Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
NO INSPECTION HAS BEEN PERFORMED ON THIS PULL REQUEST! - This PR must be inspected by setting label 'AT: PRE-TEST INSPECTED'.

kokkos-devops-admin · 2023-11-12T09:54:19Z

Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
NO INSPECTION HAS BEEN PERFORMED ON THIS PULL REQUEST! - This PR must be inspected by setting label 'AT: PRE-TEST INSPECTED'.

mzuzek mentioned this pull request Sep 8, 2022

task1: implementations NexGenAnalytics/kokkos-kernels#2

Open

mzuzek force-pushed the move-gemm-to-blas branch from 3b745b0 to ef75dc3 Compare September 9, 2022 15:25

mzuzek commented Sep 9, 2022

View reviewed changes

mzuzek marked this pull request as ready for review September 9, 2022 15:34

mzuzek mentioned this pull request Sep 14, 2022

task2: interface NexGenAnalytics/kokkos-kernels#3

Open

8 tasks

mzuzek force-pushed the move-gemm-to-blas branch from ef75dc3 to 69d97b0 Compare September 15, 2022 07:31

mzuzek force-pushed the move-gemm-to-blas branch from 69d97b0 to 2292855 Compare September 20, 2022 23:17

mzuzek force-pushed the move-gemm-to-blas branch from 2292855 to f6800db Compare September 22, 2022 13:10

mzuzek mentioned this pull request Sep 22, 2022

[WIP] GEMM: Refactor trans-mode combinations #1539

Draft

lucbv reviewed Sep 22, 2022

View reviewed changes

mzuzek force-pushed the move-gemm-to-blas branch from 34c50ea to 53c58bc Compare September 27, 2022 12:34

Mikołaj Zuzek added 7 commits September 28, 2022 11:16

GEMM: Move Serial, Team and TeamVector implementations to KokkosBlas

76f494c

GEMM: move unit tests to Blas

7ac1184

GEMM: move MKL implementation of SerialGemm to dedicated TPL header

51185f3

GEMM: connect TeamVectorGemm to the selective interface

016095b

GEMM: implicit MemberType

d945717

GEMM: bring back batched interfaces for backward compatibility

88de7f2

MKL: move utils to common header + fix macro duplication

38c9989

GEMM: fix "batched" in names

de3d26c

mzuzek force-pushed the move-gemm-to-blas branch from 53c58bc to de3d26c Compare September 28, 2022 09:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move functor-level GEMM routines to KokkosBlas #1519

Move functor-level GEMM routines to KokkosBlas #1519

mzuzek commented Sep 8, 2022 •

edited

Loading

fnrizzi commented Sep 8, 2022

kokkos-devops-admin commented Sep 8, 2022

mzuzek commented Sep 8, 2022 •

edited

Loading

mzuzek Sep 9, 2022

lucbv Sep 9, 2022

mzuzek Sep 9, 2022

lucbv Sep 9, 2022

mzuzek commented Sep 9, 2022

kokkos-devops-admin commented Sep 9, 2022

kokkos-devops-admin commented Sep 15, 2022

kokkos-devops-admin commented Sep 20, 2022

kokkos-devops-admin commented Sep 22, 2022

lucbv left a comment

lucbv Sep 9, 2022

lucbv Sep 9, 2022

mzuzek commented Sep 22, 2022

mzuzek commented Sep 27, 2022

kokkos-devops-admin commented Sep 27, 2022

kokkos-devops-admin commented Nov 12, 2023

Move functor-level GEMM routines to KokkosBlas #1519

Are you sure you want to change the base?

Move functor-level GEMM routines to KokkosBlas #1519

Conversation

mzuzek commented Sep 8, 2022 • edited Loading

fnrizzi commented Sep 8, 2022

kokkos-devops-admin commented Sep 8, 2022

mzuzek commented Sep 8, 2022 • edited Loading

mzuzek Sep 9, 2022

Choose a reason for hiding this comment

lucbv Sep 9, 2022

Choose a reason for hiding this comment

mzuzek Sep 9, 2022

Choose a reason for hiding this comment

lucbv Sep 9, 2022

Choose a reason for hiding this comment

mzuzek commented Sep 9, 2022

kokkos-devops-admin commented Sep 9, 2022

kokkos-devops-admin commented Sep 15, 2022

kokkos-devops-admin commented Sep 20, 2022

kokkos-devops-admin commented Sep 22, 2022

lucbv left a comment

Choose a reason for hiding this comment

lucbv Sep 9, 2022

Choose a reason for hiding this comment

lucbv Sep 9, 2022

Choose a reason for hiding this comment

mzuzek commented Sep 22, 2022

mzuzek commented Sep 27, 2022

kokkos-devops-admin commented Sep 27, 2022

kokkos-devops-admin commented Nov 12, 2023

mzuzek commented Sep 8, 2022 •

edited

Loading

mzuzek commented Sep 8, 2022 •

edited

Loading