usnistgov / mass

Microcalorimeter pulse-analysis software
MIT License
6 stars 0 forks source link

data.avg_pulses_auto_masks() python3 compatibility #47

Closed joefowler closed 8 years ago

joefowler commented 8 years ago

Original report by Charles James Titus (Bitbucket: cjtitus, GitHub: cjtitus).


On datasets which contain channels with no good pulses, data.avg_pulses_auto_masks() fails with IndexError: index -1 is out of bounds for axis 0 with size 0

This occurs at line 1538 of channel_groups.py:

median_pulse_avg = np.array([np.median(ds.p_pulse_average[ds.good()]) for ds in self])

On python3, np.median([]) throws an index error and crashes, whereas on python2, np.median([]) returns NaN

joefowler commented 8 years ago

Original comment by Young Il Joe (Bitbucket: jojo978, GitHub: jojo978).


Most of time I used Python 3 on Windows 7. And in this environment, np.median([]) actually gives a runtime warning and returns nan. I think that's probably a reason why I haven't had this issue on Windows.

joefowler commented 8 years ago

Original comment by Joseph Fowler (Bitbucket: joe_fowler, ).


@oneilg writes that the method leading to this error is overly complicated and needs a rewrite. The following is from his email of May 2 at 11:47 am.

I think data.avg_pulses_auto_masks() should be up for a complete re-write, because it is still overly complicated from the CDM legaacy. This should be pretty easy to do, because the algorithm is pretty simple.

As a short term fix try something like

#!python

for ds in data:
    If ds.good().sum()==0:
       data.set_chan_bad(ds.channum, "no good pulses")
joefowler commented 8 years ago

Original comment by Joseph Fowler (Bitbucket: joe_fowler, ).


While fixing data.avg_pulses_auto_masks, I should also remove the crosstalk veto option, as it applies only to CDM.

Also add option to make_masks(...) using a range of pulse_rms as well as pulse_average and peak_value.

Finally, remove the ability to make multiple pulse averages using a sequence of sequences of masks. Too insanely complex, and we have never found use for it.

joefowler commented 8 years ago

Original comment by Charles James Titus (Bitbucket: cjtitus, GitHub: cjtitus).


This may not matter if you plan to re-write the method, but I do want to mention that with Galen's fix of setting channels bad if they have no good pulses, I still get an error. In this traceback, mask[19] is actually the position of the first bad channel, so I'm wondering if there are two loops somewhere, and one excludes bad channels but one includes them. I haven't tracked this down further, but I can look into the root cause later. But if you're going to re-write the thing, it's something else to be aware of.

#!python

ValueError                                Traceback (most recent call last)
/home/jamie/work/irwin_group/ssrl_tes_analysis/scripts/20160417/20160417_005_analysis.py in <module>()
     79         data.set_chan_bad(ds.channum, "too few good pulses")
     80 
---> 81 data.avg_pulses_auto_masks(forceNew=False)  # creates masks and compute average pulses
     82 #data.plot_average_pulses(-1)
     83 data.compute_filters(f_3db=20000.0, forceNew=False)

/home/jamie/anaconda3/lib/python3.4/site-packages/mass-0.4.2-py3.4-linux-x86_64.egg/mass/core/channel_group.py in avg_pulses_auto_masks(self, max_pulses_to_use, forceNew)
   1541             if len(m) > max_pulses_to_use:
   1542                 m[max_pulses_to_use:] = False
-> 1543         self.compute_average_pulse(masks, forceNew=forceNew)
   1544 
   1545     def drift_correct(self, forceNew=False, category=None):

/home/jamie/anaconda3/lib/python3.4/site-packages/mass-0.4.2-py3.4-linux-x86_64.egg/mass/core/channel_group.py in compute_average_pulse(self, masks, subtract_mean, forceNew)
   1131                         if mask.shape != (chan.nPulses,):
   1132                             raise ValueError("\nmasks[%d] has shape %s, but it needs to be (%d,)" %
-> 1133                                              (imask, mask.shape, chan.nPulses))
   1134                         if len(valid) > chan.data.shape[0]:
   1135                             good_pulses = chan.data[valid[:chan.data.shape[0]], :]

ValueError: 
masks[19] has shape (122,), but it needs to be (18296,)
joefowler commented 8 years ago

Original comment by Joseph Fowler (Bitbucket: joe_fowler, ).


Jamie: I've been re-writing the code this morning, and indeed I noticed that there would probably be a bug of exactly the form you're pointing out. Yes, there are incompatible loops, some over ALL detectors and some over all GOOD detectors.

Since Galen's fix was of no avail, I'll make sure to get this fix pushed to the develop branch soon. First I need to make sure that the fix works on data sets with one or more channels flagged bad.

joefowler commented 8 years ago

Original comment by Joseph Fowler (Bitbucket: joe_fowler, ).


Fix bugs in avg_pulses_auto_masks().

Fixes #47.