ExCALIBUR-NEPTUNE / NESO

MIT License
4 stars 4 forks source link

[WIP] Using `std::normal_distribution` in place of inverse transform sampling #203

Open matt-graham opened 1 year ago

matt-graham commented 1 year ago

Description

Would fix #196.

Changes manual implementation of inverse transform sampling using boost::math::erf_inv to generate normally distributed random variates when sampling thermally distributed initial particle velocities to instead use the generator implementation for std::normal_distribution. Microbenchmarking suggests this is slightly more performant, but the main gain is that the intent of the code is (hopefully) clearer.

Opening this a draft PR at the moment as I haven't be able to build locally to test due to #202 and ExCALIBUR-NEPTUNE/NESO-Particles/issues/37

Type of change

Testing

Please describe the tests that you ran to verify your changes and provide instructions for reproducibility. Please also list any relevant details for your test configuration.

Test Configuration:

Checklist:

oparry-ukaea commented 1 year ago

Hi @matt-graham, @cmacmackin, Any reason this can't be merged now?

matt-graham commented 1 year ago

Hi @oparry-ukaea. Sorry this dropped off my radar a bit as I had issues with trying to get NESO to build (using Spack) to run the test suite against the changes locally. Other than this wasn't any other reason for this still being draft from my perspective.

matt-graham commented 1 year ago

I've just rebased against current main and tried building using Spack but still hitting against issue described in #202

oparry-ukaea commented 1 year ago

Is this building in a spack/ubuntu-jammy container? If so, which version?