DM-39460: Bug-fix for CHARGE_SUM photodiode integration method #266
Conversation
python/lsst/ip/isr/photodiode.py
Outdated
| index = np.where(current < dy/20. + np.min(current)) | ||
| bg_current = np.sum(charge[index])/np.sum(dt[index]) | ||
| signal = np.where(current > dy/20. + np.min(current)) | ||
| imin = signal[0][0] - 2 |
There was a problem hiding this comment.
What happens if the first point is at the edge? Do we need to make sure this doesn't go below 0? (Similar for the next line at the max).
There was a problem hiding this comment.
Both list(range(0, imin)) and list(range( imax, len(current))) return empty lists when imin < 0 and imax > len(current), respectively, which is what we want, I think. Same appears to be true for np.arange.
python/lsst/ip/isr/photodiode.py
Outdated
| signal = np.where(current > dy/20. + np.min(current)) | ||
| imin = signal[0][0] - 2 | ||
| imax = signal[0][-1] + 2 | ||
| bg = (np.array(list(range(0, imin)) + list(range(imax, len(current)))),) |
There was a problem hiding this comment.
If I'm reading this correctly, this is a tuple of an array of a list ....
This should be simpler and I think should work the same. There's no need for the 1d tuple:
bg = np.concatenate([np.arange(0, imin), np.arange(imax, len(current))])
python/lsst/ip/isr/photodiode.py
Outdated
| dy = np.max(current) - np.min(current) | ||
| index = np.where(current < dy/20. + np.min(current)) | ||
| bg_current = np.sum(charge[index])/np.sum(dt[index]) | ||
| signal = np.where(current > dy/20. + np.min(current)) |
There was a problem hiding this comment.
If this is a 1d array, you can do signal, = np.where(current > dy/20. + np.min(current)) and just get the indices and below you can do signal[0] and signal[-1]. And although this is what I still do, the numpy recommendation is instead to do signal, = (current > dy/20. + np.min(current)).nonzero().
|
@erykoff I've implemented your suggestions and added some test code for that failure mode. |
No description provided.