Skip to content

Fix incorrect CPU float16 implementation#2395

Open
eliasachermann wants to merge 4 commits into
spcl:mainfrom
eliasachermann:fix/cpu-half-conversion
Open

Fix incorrect CPU float16 implementation#2395
eliasachermann wants to merge 4 commits into
spcl:mainfrom
eliasachermann:fix/cpu-half-conversion

Conversation

@eliasachermann

Copy link
Copy Markdown

Problem

The dace::half fallback in dace/runtime/include/dace/types.h is broken in two independent ways:

  1. No default constructor.
    Generated code that declares an uninitialized dace::float16 fails to compile:

    error: no matching function for call to 'dace::half::half()'
    
  2. Incorrect conversions.
    The float↔half conversion is wrong for zero, subnormals, Inf/NaN, and does no rounding.

Fix

  • Replace both conversions with IEEE-754 round-to-nearest-even routines, correctly handling zero, subnormals, overflow→Inf and Inf/NaN.
  • Add a default constructor.

Tests

Adds tests/half_cpu_test.py, to test the conversion against NumPy as the reference.

@eliasachermann eliasachermann changed the title correct CPU half (float16) conversion, add default constructor Fix incorrect CPU float16 implementation Jun 6, 2026
Comment thread tests/half_cpu_test.py Outdated
Remove print statement indicating all tests passed.
@ThrudPrimrose ThrudPrimrose self-requested a review June 8, 2026 16:18

@ThrudPrimrose ThrudPrimrose left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We need subnormal, inf and NaN in roundtrip tests to ensure we handle them correctly.

Comment thread tests/half_cpu_test.py
1e-3,
6e-5,
6e-8,
-6e-8,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you add a subnormal number to your tests?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added inf and nan tests

@acalotoiu acalotoiu 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.

LGTM

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.

3 participants