[WIP] Replace sin(theta), cos(theta) with amrex::Math::sincos(theta) when appropriate

[lucafedeli88]

This PR mirrors what was done for ImpactX in ECP-WarpX/impactx#493 by @ax3l

[ax3l]

Thanks, LGTM! 👍

Note that as linked in the other PR, and tested when I added the function to AMReX, most optimizing compilers should find this automatically. But this way it is done also in all optimization levels, and cleaner if the results are needed multiple times.

see qs

[ax3l]

Restarting CI, all Azure tests failed...?

[ax3l]

CI still not happy, huh. Can you please rebase?

[RevathiJambunathan]

cool! Thanks for this PR, @lucafedeli88

[ax3l]

Looks like CI is not happy.

Can you rebase @lucafedeli88 ?

[ax3l]

There seem to be percent-level changes for momenta in checksum files 🤔

[ax3l]

@lucafedeli88 I think there might be a bug here if the results change.

Potentially, one could systematically pick one test and bisect which file change introduces the differences?

[ax3l]

@lucafedeli88 can you rebase this?
Potentially, it is worth submitting this piece wise if it is hard to spot which line changes cause the checksum changes.

[lucafedeli88]

@lucafedeli88 can you rebase this? Potentially, it is worth submitting this piece wise if it is hard to spot which line changes cause the checksum changes.

@ax3l , I've merged the newest version of WarpX into this branch. I'll try to understand which substitution is causing the bug.

[lucafedeli88]

@lucafedeli88 can you rebase this? Potentially, it is worth submitting this piece wise if it is hard to spot which line changes cause the checksum changes.

@ax3l , I've merged the newest version of WarpX into this branch. I'll try to understand which substitution is causing the bug.

I am testing the hypothesis that the small discrepancies come from using sincospi instead of sincos(pi*arg). The former should be more precise (sincospi does not use pi internally), while sincos(pi*arg) uses pi in single or double precision. This could explain discrepancies at roughly machine precision. In case this hypothesis turns out to be correct, I think that we should switch to sincospi, since it is more precise and more performant.

[lucafedeli88]

Actually not all the discrepancies are small, so maybe there's something else.

[lucafedeli88]

Implementing this feature without introducing discrepancies seems to be very difficult, and I am not sure that I will have time to work on that on the foreseeable future. Since the priority of this feature is very low, I will close the PR.