Skip to content

Bugfix in TPCFastTransform with reduced metadata#141

Merged
davidrohr merged 3 commits intodavidrohr:dev_pull_request6from
sgorbuno:PR21.04
Apr 21, 2026
Merged

Bugfix in TPCFastTransform with reduced metadata#141
davidrohr merged 3 commits intodavidrohr:dev_pull_request6from
sgorbuno:PR21.04

Conversation

@sgorbuno
Copy link
Copy Markdown

Hi David, Ruben.

@davidrohr, @shahor02

This PR fixes the commit bd7be4f9a69b37b50ee0fcd3c94b58a6b346d4cb
"TPCFastTransform: reduce the metadata: the same spline setup for all the sectors"

The TPCFastSpaceChargeCorrection and the TPCFastTransformPOD now show the same results everywhere on the CPU.
I didn't test it on the GPU, but it must work. David, please check.

There is a little difference (of the order of 1.e-6 at max) in getCorrectionYZatRealYZ(..) between the POD and the standard version of the SC correction.

I assume the difference is due to differences in the code optimization.

This difference was not there before. It appears right after the commit b90942c92ed3bc599e157472db21585069483325
"TPCFastTransform: remove row-wise max correction values"

It disappears again when I disable the check for the large values in getCorrectionYZatRealYZ() by commenting out the corresponding lines:

https://github.com/sgorbuno/AliceO2/blob/PR21.04/GPU/TPCFastTransformation/TPCFastSpaceChargeCorrection.h#L500
https://github.com/sgorbuno/AliceO2/blob/PR21.04/GPU/TPCFastTransformation/TPCFastTransformPOD.h#L362

Interesting: in my test, the check for large values is never triggered. But just the presence of this check in the code changes the calculations, apparently.

@sgorbuno sgorbuno requested a review from davidrohr as a code owner April 20, 2026 21:49
@davidrohr
Copy link
Copy Markdown
Owner

fix works, thx a lot.
Well, by default we compile with -ffast-math, so changes in the code can change the results slightly

@davidrohr davidrohr merged commit ee8ab28 into davidrohr:dev_pull_request6 Apr 21, 2026
5 of 6 checks passed
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