GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — GPU: make Metal's cluster widths round as on the CPU by ktf · Pull Request #16065 · AliceO2Group/AliceO2

Conversation

@ktf

@ktf ktf commented Nov 6, 2026

Copy link
Copy Markdown
Member

Needs review. On a 30-collision Pb-Pb time frame, 7 of 4,416,506 clusters
differed between the CPU and the Metal backend, each by one step of a
packed width, sigmaTime or sigmaPad. Those widths were all within one
float ulp below a quantisation step: sigma * 32 was 11.4999994,
14.4999992, 29.4999996 or 31.4999988, where the packing rounds at n + 0.5,
so any difference in the last bit decides between n and n + 1.

The CPU build compiles this with -ffast-math. Its arm64 code for
finalize() computes 1 / Q once, the means as products with it, the
variance as one fused multiply-add, fma(SS, 1 / Q, -mean * mean), and an
IEEE sqrt. Metal's fast-math division and sqrt are not correctly rounded.
Do the same operations on Metal explicitly, with precise::divide, fma and
precise::sqrt: the pbpb clusters then match the CPU bit for bit, with no
measurable cost; the rest of the Metal code keeps fast math.

This matches the CPU build that the compiler produces on arm64 with
clang. An x86 build, or another compiler, may choose a different sequence
under -ffast-math. Writing the sequence explicitly for all backends would
make the widths independent of those choices, but can change the CPU
results on some platforms.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@ktf
GPU: make Metal's cluster widths round as on the CPU
c967b40
Needs review. On a 30-collision Pb-Pb time frame, 7 of 4,416,506 clusters
differed between the CPU and the Metal backend, each by one step of a
packed width, sigmaTime or sigmaPad. Those widths were all within one
float ulp below a quantisation step: sigma * 32 was 11.4999994,
14.4999992, 29.4999996 or 31.4999988, where the packing rounds at n + 0.5,
so any difference in the last bit decides between n and n + 1.

The CPU build compiles this with -ffast-math. Its arm64 code for
finalize() computes 1 / Q once, the means as products with it, the
variance as one fused multiply-add, fma(SS, 1 / Q, -mean * mean), and an
IEEE sqrt. Metal's fast-math division and sqrt are not correctly rounded.
Do the same operations on Metal explicitly, with precise::divide, fma and
precise::sqrt: the pbpb clusters then match the CPU bit for bit, with no
measurable cost; the rest of the Metal code keeps fast math.

This matches the CPU build that the compiler produces on arm64 with
clang. An x86 build, or another compiler, may choose a different sequence
under -ffast-math. Writing the sequence explicitly for all backends would
make the widths independent of those choices, but can change the CPU
results on some platforms.
@ktf
ktf requested a review from davidrohr as a code owner November 6, 2026 14:49
@ktf

ktf commented Nov 7, 2026

Copy link
Copy Markdown
Member Author

@davidrohr any objections to this? It makes the both the benchmark and the FST report the same clusters independently of running on the CPU or on Metal on macOS.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@davidrohr

davidrohr commented Nov 7, 2026

Copy link
Copy Markdown
Collaborator

To be honest, I don't really see the purpose and I don't like complicating the code. IMHO, fast math rounding is anyway undefined, so you are fixing something that just works by chance compared to clang CPU results. It can break again with any new clang version / different compiler / different GPU device.
I would not try to compare in fast-math mode, but I'd try to make the comparison in the deterministic mode work. Because without it, the number of tracks will be different in any case. And for the deterministic mode, this PR won't be needed, since deterministic mode cannot use fast-math.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ktf ktf mentioned this pull request Nov 7, 2026
GPU Metal: also run the border-range sorts on the host #16071
Open
@ktf

ktf commented Nov 7, 2026

Copy link
Copy Markdown
Member Author

Ok, I will keep it only in my setup then, while I do the bring-up of the Metal version. Excluding differences keeps my friends less confused. Closing.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ktf ktf closed this Nov 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidrohr davidrohr Awaiting requested review from davidrohr davidrohr is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@ktf @davidrohr
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.