s2fft icon indicating copy to clipboard operation
s2fft copied to clipboard

PyTorch gradient tests are taking up a lot of test time

Open matt-graham opened this issue 7 months ago • 1 comments

I noticed as part of working on #277 that the gradient checks on the PyTorch implementations of the precompute transforms are very slow and infact end up constituting a significant proportion of the overall test suite run time. With these checks removed I can run the whole test suite locally, distributing across 4 processes with pytest_xdist, in 7 minutes, compared to 55 minutes with these checks included.

Given how long these checks take, it might make sense to factor them out in to separate tests and apply a mark to them so they can be skipped when running the tests on pull requests and only run them when merging to main and in the scheduled runs.

https://github.com/astro-informatics/s2fft/blob/1d5fa15ac0233054542178c1a37087abce0770b6/tests/test_spherical_precompute.py#L74-L88

https://github.com/astro-informatics/s2fft/blob/1d5fa15ac0233054542178c1a37087abce0770b6/tests/test_spherical_precompute.py#L135-L149

https://github.com/astro-informatics/s2fft/blob/1d5fa15ac0233054542178c1a37087abce0770b6/tests/test_spherical_precompute.py#L203-L217

https://github.com/astro-informatics/s2fft/blob/1d5fa15ac0233054542178c1a37087abce0770b6/tests/test_spherical_precompute.py#L267-L283

https://github.com/astro-informatics/s2fft/blob/1d5fa15ac0233054542178c1a37087abce0770b6/tests/test_wigner_precompute.py#L61-L75

https://github.com/astro-informatics/s2fft/blob/1d5fa15ac0233054542178c1a37087abce0770b6/tests/test_wigner_precompute.py#L122-L136

https://github.com/astro-informatics/s2fft/blob/1d5fa15ac0233054542178c1a37087abce0770b6/tests/test_wigner_precompute.py#L178-L193

https://github.com/astro-informatics/s2fft/blob/1d5fa15ac0233054542178c1a37087abce0770b6/tests/test_wigner_precompute.py#L237-L252

matt-graham avatar Apr 11 '25 13:04 matt-graham

Thanks for catching @matt-graham ! Yes, totally agree we factor these into "nightly" tests (which we probably run weekly?) and not part of the local test suit to save time when running tests locally and for PR merges.

jasonmcewen avatar Apr 23 '25 09:04 jasonmcewen