search

desihub / specter

A toolkit for simulating multi-object spectrographs
Other
8 stars 7 forks source link

Legval speedup #61

Closed lastephey closed 6 years ago

lastephey commented 6 years ago

This pull request adds an optional numba-ized version of legval (one of the most important kernels in the code). For a test case of desi_extract_spectra this results in about a 20 percent overall speedup on KNL and Haswell. If numba is not present, the code will fall back to the original numpy legval.

sbailey commented 6 years ago

Thanks. Please add some unit tests that validate that legval_numba and numpy.polynomial.legendre.legval return the same values for various sizes of x and c, including the cases where x or c are scalars instead of arrays. numpy.allclose is sufficient for comparison testing if they aren't an exact bitwise match.

lastephey commented 6 years ago

Added unit tests for several sizes of x and c. We assume c must be an array with len(c) >2, so there are no tests in which c is a scalar.

sbailey commented 6 years ago

To keep ourselves on the straight-and-narrow with licensing, I changed this to falling back to importing numpy.polynomial.legendre.legval instead of directly copying the code into our repo (which would have required us also copying the license). Same effect in the end: if numba is installed, things run faster, but if it isn't then it falls back to the numpy version of legval. Ready to merge when tests re-pass.