search

desihub / specter

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

fix legval_numba scalar vs. vectors #62

Closed sbailey closed 6 years ago

sbailey commented 6 years ago

On both Cori and my laptop, I was getting numba errors on specter.psf.spotgrid.new_pixshift. Using vanilla @numpy.jit works, but @numba.jit(nopython=True, cache=True) does not. @lastephey is away this week so this is an emergency patch and I'll coordinate with her next week about testing nopython vs. not.

This is a schroedinbug: once observed it became reproducible, but I don't understand how it ever worked in the first place or how testing didn't catch it. This code was introduced 5 months ago in PR #56 and tests apparently only started failing while I was on vacation last week. The Travis tests were failing on "import numba" before ever running, which Travis incorrectly interpreted as success (!), but I don't understand how/why the nightly NERSC tests weren't catching this.

A few other changes came along for the ride:

sbailey commented 6 years ago

I don't fully understand what went wrong, but at least a big part of it was that PR #61 altered TraceSet.eval such that it worked with vectors but not scalars. I think this manifested itself by breaking the code introduced months ago in #56, leading to the confusion about how that PR ever worked. This was exacerbated by Travis tests that were failing in a way that made Travis report that everything was passing, and possibly also by numba caching issues. I'll correct the PR title since nopython=True turned out to not be the culprit.

This branch now passes on Travis (for real), cori, and my laptop, so I'll proceed with merging. I'll still followup with @lastephey next week for testing if these changes introduced a performance regression on what was improved in PR #56 and #61.