Skip to content

Support for half_t as a scalar type - #3360

Open
brian-kelley wants to merge 21 commits into
kokkos:developfrom
brian-kelley:fp16
Open

brian-kelley wants to merge 21 commits into
kokkos:developfrom
brian-kelley:fp16

Conversation

@brian-kelley

Copy link
Copy Markdown
Contributor
  • CMake logic to check if half_t is a natively supported type according to kokkos, and if so provide the option KokkosKernels_INST_HALF
    • if not natively supported (half_t == float) then this option can't be turned on
  • A few fixes to library code for half_t. For example, don't pass a half_t to printf directly, and don't try to implicitly convert a bool to half_t.

But a large majority of the changes are for enabling half_t coverage in tests:

  • instantiate tests for half_t
  • increase numerical tolerances for half_t. This introduces a helper TestUtils::selectEpsilonForScalarType<Scalar>(eps_double, eps_float, eps_half) to make this cleaner
  • in some places, decrease the size of the matrix/vector for half_t test cases to reduce precision loss
  • In ode, the bdf test only runs bdf1 and skips the convergence order check

lucbv and others added 16 commits October 19, 2026 13:50
A few things need to happen, first we need to enable the logic
in cmake, second we need to add some guard to only enable when
half is not float otherwise we create double instantiations for
the float type, thirds we will likely need to clean up some code
in the library...

Signed-off-by: Luc Berger-Vergiat <lberge@sandia.gov>
Now using the macro defined in Kokkos Core to signal support of half_t
using native type instead of float.
Adding test that exercise the half code path when the type is instantiated.

Signed-off-by: Luc Berger-Vergiat <lberge@sandia.gov>
Signed-off-by: Luc Berger-Vergiat <lberge@sandia.gov>
Signed-off-by: Luc Berger-Vergiat <lberge@sandia.gov>
Co-Authored-By: Laguna S2.1
Signed-off-by: Brian Kelley <bmkelle@sandia.gov>
Make sure compiler launcher from Kokkos is used with
check_cxx_symbol_exists, otherwise this test doesn't work for cuda.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Brian Kelley <bmkelle@sandia.gov>
batched dense/sparse, blas serial/team, most of sparse

Co-Authored-By: Laguna S 2.1
Signed-off-by: Brian Kelley <bmkelle@sandia.gov>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Brian Kelley <bmkelle@sandia.gov>
Signed-off-by: Brian Kelley <bmkelle@sandia.gov>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Brian Kelley <bmkelle@sandia.gov>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Brian Kelley <bmkelle@sandia.gov>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Brian Kelley <bmkelle@sandia.gov>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Brian Kelley <bmkelle@sandia.gov>
Signed-off-by: Brian Kelley <bmkelle@sandia.gov>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Brian Kelley <bmkelle@sandia.gov>
Signed-off-by: Brian Kelley <bmkelle@sandia.gov>
@brian-kelley brian-kelley changed the title Full support for half_t as a scalar type Support for half_t as a scalar type Oct 23, 2026
brian-kelley and others added 2 commits October 23, 2026 18:47
and make nrm2w_squared test for half_t consistent with nrm2w

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Brian Kelley <bmkelle@sandia.gov>
Signed-off-by: Brian Kelley <bmkelle@sandia.gov>

@lucbv lucbv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think that everything needs to be addressed, but some of the comment should be turned into issues to work on later. Will do that when I get to the office...

Comment on lines +28 to +29
A[i * as] += ValueType(minus_abs_tiny) * ValueType(static_cast<float>(a_real < 0));
A[i * as] += ValueType(abs_tiny) * ValueType(static_cast<float>(a_real >= 0));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be curious to see if there is any performance reason not to implement this as a tertiary operator?

Suggested change
A[i * as] += ValueType(minus_abs_tiny) * ValueType(static_cast<float>(a_real < 0));
A[i * as] += ValueType(abs_tiny) * ValueType(static_cast<float>(a_real >= 0));
A[i * as] += (a_real < 0) ? ValueType(minus_abs_tiny) : ValueType(0);
A[i * as] += (a_real >= 0) ? ValueType(abs_tiny) * ValueType(0);

// and half_t is not an alias to float.
#if !defined(KOKKOSKERNELS_ETI_ONLY) && !defined(KOKKOSKERNELS_IMPL_CHECK_ETI_CALLS) && \
defined(KOKKOS_HALF_T_IS_FLOAT) && !KOKKOS_HALF_T_IS_FLOAT
#if !defined(KOKKOSKERNELS_ETI_ONLY) && !defined(KOKKOSKERNELS_IMPL_CHECK_ETI_CALLS) && defined(KOKKOSKERNELS_INST_HALF)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, we are instantiating half_t now so we could probably revisit this logic?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For laswp that does not compute any arithmetic operations I would have expected that we can use the arithmetic traits epsilon value?

#pragma unroll
#endif
for (int i = 0; i < m; ++i) A[i * as0] *= alpha;
for (int i = 0; i < m; ++i) A[i * as0] *= ValueType(alpha);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might not be really what we want especially if ScalarType is double and ValueType is lower precision?
This goes back to discussions about the definition of our rules for type promotion...


template <class ScalarA, class ScalarB, class Device>
int test_dot() {
const int Nbig = std::is_same<ScalarA, Kokkos::Experimental::half_t>::value ? 128 : 1024;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the problem that we cannot add too many random generated numbers before risking overflow with half_t?
I am suspecting that we are doing the reduction with double by default which should be fine for rounding error but might not be possible to cast without overflow. That's something that we could test for in the algorithm itself in debug mode...

@brian-kelley brian-kelley Oct 27, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, we don't have an InnerProductSpaceTraits or DotAccumulatingScalar specialization for half_t, so this was doing the accumulation in half_t. I'll try changing this to accumulate in float and then see if we can get away with the same vector length as other tests.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like we talked about yesterday, native dot was not accumulating in double (even for float vectors). I opened #3365 to fix this. If possible I'd like to get that merged first

Comment on lines +462 to +464
const int max_x = 1;
const int max_y = 1;
const int max_val = 1;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

Comment on lines +546 to +548
const int max_x = 10;
const int max_y = 10;
const int max_val = 10;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

Comment on lines +614 to +616
const int max_x = 10;
const int max_y = 10;
const int max_val = 10;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

Comment on lines +705 to +707
const int max_x = 1;
const int max_y = 1;
const mag_t max_val = static_cast<mag_t>(2);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even more suspicious...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to promote the use of KokkosKernels::ArithTraits<>::one()/zero() to our LLM friends

brian-kelley and others added 3 commits October 26, 2026 10:43
Signed-off-by: Brian Kelley <bmkelle@sandia.gov>
Signed-off-by: Brian Kelley <bmkelle@sandia.gov>
- Fix selectEpsilonForScalarType; if KOKKOS_HALF_T_IS_FLOAT it
should use the float value (2nd arg) when scalar_t is either
half_t or float, not the half value (3rd arg).
- Fix tolerances in ODE Newton tests (were wrong when
KOKKOS_HALF_T_IS_FLOAT for same reason).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Brian Kelley <bmkelle@sandia.gov>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants