desihub / fastspecfit

Fast spectral synthesis and emission-line fitting of DESI spectra.
https://fastspecfit.readthedocs.org
BSD 3-Clause "New" or "Revised" License
13 stars 2 forks source link

refactor fitting engine to not use fnnls or astropy.modeling #92

Closed moustakas closed 1 year ago

moustakas commented 1 year ago

[WIP]

Still WIP but opening a PR to track related issues.

This PR is a fairly major rewrite of the emission-line fitting engine with an eye toward porting the code to the Perlmutter/GPUs (e.g., https://jaxopt.github.io/stable/constrained.html). Specifically, all the astropy.modeling routines have been replaced with a simpler table-based linemodel of parameters (including tied parameters), resulting in about 300 fewer lines of code and notable speed-ups.

However, the speed-ups have been used to include one additional (third) round of fitting where emission lines are minimally constrained to one another, which leads to notable improvements in the line-fitting results for systems with complex line-kinematics (and to account for any cross-camera wavelength-calibration issues).

Second, I've moved away from the home-grown fnnls algorithm for continuum fitting and am now just using scipy.optimize.nnls which I also hope to be able to port to a GPU.

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 3809189964


Changes Missing Coverage Covered Lines Changed/Added Lines %
py/fastspecfit/io.py 96 111 86.49%
py/fastspecfit/continuum.py 55 130 42.31%
py/fastspecfit/emlines.py 495 577 85.79%
py/fastspecfit/fastspecfit.py 24 508 4.72%
<!-- Total: 670 1326 50.53% -->
Files with Coverage Reduction New Missed Lines %
py/fastspecfit/fastspecfit.py 1 21.27%
py/fastspecfit/emlines.py 6 64.75%
<!-- Total: 7 -->
Totals Coverage Status
Change from base Build 3522749933: -12.2%
Covered Lines: 2049
Relevant Lines: 3539

💛 - Coveralls
moustakas commented 1 year ago

This icicle graph shows the original code (using astropy.modeling:

Screen Shot 2022-12-26 at 9 25 00 AM

And here's with the refactored code (including the third round of fitting), where I'm fairly certain I can shave off more time in _build_emline_model using numba.jit.

Screen Shot 2022-12-26 at 9 25 28 AM
moustakas commented 1 year ago

The line-fitting portion of this PR has been tested fairly extensively but will need additional development. In addition, the work on the continuum-fitting engine will require new templates, but I'm going to defer that work to a different PR. Merging now.