inconsistent default size type between `CrsMatrix` and `StaticCrsGraph`

[maartenarnst]

@brian-kelley @romintomasetti

In the following PR:

• https://github.com/kokkos/kokkos-kernels/pull/2149

the default size types of KokkosSparse::CrsMatrix and several other matrix types were changed to default_size_type. This is typically int or size_t depending on the configuration of Kokkos-kernels.

However, in Kokkos::StaticCrsGraph, the default size type is taken from a ViewTraits instance. Typically, that size type is in turn a size type of a space, and it need not be int or size_t.

• https://github.com/kokkos/kokkos/blob/a14a5d9bee2f84e1b160df203ed2dd6c8f2060c3/containers/src/Kokkos_StaticCrsGraph.hpp#L256-L260

The issue is thus that these default size types are not necessarily the same. And because KokkosSparse::CrsMatrix and Kokkos::StaticCrsGraph are so interrelated, this discrepancy can lead to problems when using eg. the KokkosSparse::CrsMatrix constructor that takes a Kokkos::StaticCrsGraph among its arguments.

This issue is thus a suggestion to consider whether further consistency across the size types may be worth pursuing.

[maartenarnst]

@lucbv

Looks like it will be solved :)

[lucbv]

Yes, it was good timing you reminded me about this, I was able to "jump on it" effectively during the meeting. Since we are the biggest user of this container I think it is reasonable to move it : )

[MalachiTimothyPhillips]

This is still an issue in user land. I just encountered a Sierra use-case that required explicitly specifying the size type to avoid a compilation error in code that previously compiled without issue.

struct SparseMatrix {
  using execution_space = Kokkos::DefaultExecutionSpace;
  using mat_type = KokkosSparse::CrsMatrix<double, int, execution_space>;

  SparseMatrix() = default;
  SparseMatrix(const std::string& label,
               int numCols,
               const std::vector<double>& values,
               std::vector<int> rowIds,
               const std::vector<std::vector<int>>& rowMap)
      : globalRows(std::move(rowIds)) {
    using GraphType = Kokkos::StaticCrsGraph<int, execution_space>;
    auto sparseGraph =
        Kokkos::create_staticcrsgraph<GraphType>("transformationMatrixGraph", rowMap);
    Kokkos::View<const double*, Kokkos::HostSpace> valsH(values.data(), values.size());
    Kokkos::View<double*> valsD("transformationMatrix vals", valsH.extent(0));
    Kokkos::deep_copy(valsD, valsH);
    Kokkos::fence();

    matrix = mat_type("transformationMatrix", numCols, valsD, sparseGraph);
  }

  int numRows() const { return matrix.numRows(); }

  int numCols() const { return matrix.numCols(); }

  auto rowConst(const mat_type::ordinal_type i) const { return matrix.rowConst(i); }

  mat_type matrix;
  std::vector<int> globalRows;
};

to:

struct SparseMatrix {
  using execution_space = Kokkos::DefaultExecutionSpace;
  using GraphType = Kokkos::StaticCrsGraph<int, execution_space>;
  using mat_type = KokkosSparse::CrsMatrix<double, int, execution_space, void, GraphType::size_type>;

  SparseMatrix() = default;
  SparseMatrix(const std::string& label,
               int numCols,
               const std::vector<double>& values,
               std::vector<int> rowIds,
               const std::vector<std::vector<int>>& rowMap)
      : globalRows(std::move(rowIds)) {
    auto sparseGraph =
        Kokkos::create_staticcrsgraph<GraphType>("transformationMatrixGraph", rowMap);
    Kokkos::View<const double*, Kokkos::HostSpace> valsH(values.data(), values.size());
    Kokkos::View<double*> valsD("transformationMatrix vals", valsH.extent(0));
    Kokkos::deep_copy(valsD, valsH);
    Kokkos::fence();

    matrix = mat_type("transformationMatrix", numCols, valsD, sparseGraph);
  }

  int numRows() const { return matrix.numRows(); }

  int numCols() const { return matrix.numCols(); }

  auto rowConst(const mat_type::ordinal_type i) const { return matrix.rowConst(i); }

  mat_type matrix;
  std::vector<int> globalRows;
};

[brian-kelley]

@maartenarnst @MalachiTimothyPhillips Agree that the defaults should match. Before this was more difficult since StaticCrsGraph lived in Kokkos, but now it lives in KokkosKernels. #2553 makes it use the same default (KokkosKernels::default_size_type) as all the matrix types.