Skip to content

Conversation

@RChrHill
Copy link
Contributor

Add Sp(4) versions of DeoFlops benchmarks.
There is only a double-precision version due to a bug in the Sp(4) Grid implementation causing FP32 code to call into FP64 template specialisations.

(Thanks to Ed and Alexis for providing the implementation -- marked as draft since I want to do a small amount of harmonisation with the rest of the codebase)

@RChrHill RChrHill force-pushed the feature/sp4-deo-flops branch from d07d7b7 to 7624640 Compare November 19, 2025 02:15
@RChrHill RChrHill marked this pull request as ready for review November 19, 2025 02:17
@aportelli
Copy link
Owner

Hi @RChrHill thanks! I am just appreciating the only FP64 aspect. On can live with it but it is quite awkward to have different benchmarks for different precisions. How much time would be needed to correct this in Grid?

We could have the benchmark logic pretending this will be fixed and having a warning meanwhile.

@RChrHill RChrHill force-pushed the feature/sp4-deo-flops branch from 254a2a1 to 49ff1bd Compare November 19, 2025 18:12
@RChrHill
Copy link
Contributor Author

RChrHill commented Nov 19, 2025

Hi @aportelli, I've submitted a fix for the FP32 version to Grid. While we wait for it to be merged, I've committed a workaround in 49ff1bd.

The general problem is the Sp<4>::HotConfiguration doesn't work for FP32 gauge fields, due to the issue in the Grid PR. So, I propose that we work around this by detecting an Sp4 single-precision instantiation and generate the gauge field in FP64, then just cast it to FP32.

Also caught two FP32 templates instantiated as FP64, thanks to logging the Action's precision with the compile-time metadata structs...

Attached is an example output JSON for the flops results (hacked to only run on 8^4 and 12^4 locally): result.json

@aportelli
Copy link
Owner

I understand. The random gauge field is essentially irrelevant to the benchmarking, so this should not be a constraint here. For the moment I am happy with the workaround you proposed, any other way to initialised the field would be fine as well.

@aportelli aportelli merged commit edce6ac into aportelli:main Nov 20, 2025
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