Skip to content

Fix float16 inequality comparing raw bits as an integer - #529

Closed
taking-lying-flat wants to merge 1 commit into
pytorch:mainfrom
taking-lying-flat:fix/float16-inequality
Closed

taking-lying-flat wants to merge 1 commit into
pytorch:mainfrom
taking-lying-flat:fix/float16-inequality

Conversation

@taking-lying-flat

Copy link
Copy Markdown

float16::operator!= currently passes rhs.x to operator==. This selects the integer overload and converts the right operand's raw bit pattern as a numeric integer, instead of comparing the two float16 values.

For example:

const gloo::float16 one(1);
const gloo::float16 numericBits(15360); // 15360 is the raw bit pattern of half(1).

one == one;         // true
one != one;         // currently true; should be false
numericBits == one; // false
numericBits != one; // currently false; should be true

Pass rhs itself to the existing equality operator. This one-line change makes inequality its logical complement and preserves the current equality semantics, including its treatment of special bit patterns.

Validation against the actual modified header, compiled with GCC 15.2.0, C++20 and -O2:

  • Exhaustively checked all 65,536 bit patterns for (a != a) == !(a == a): 65,533 inconsistencies before, zero after.
  • Checked 100,000 deterministic pseudorandom pairs for inequality/equality complementarity and inequality symmetry: all passed after the fix.
  • Both concrete examples above now return the expected results.
  • clang-format 18.1.8 --dry-run --Werror gloo/types.h and git diff --check passed.

Only this production line is changed; no test files are included. The comparison checks ran on CPU; the full repository suite and CUDA build were not run.

@taking-lying-flat

Copy link
Copy Markdown
Author

Consolidated into #527 to keep these small Gloo correctness fixes in one review. The same one-line float16 inequality fix is included there as a separate commit, with the comparison checks rerun on the combined version. Closing this duplicate PR.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant