MideTechnology / idelib

Python API for accessing .IDE data recordings made by enDAQ devices.
MIT License
5 stars 0 forks source link

Remove or modify EventArray._getBlockIndexWith*() methods #22

Open StokesMIDE opened 3 years ago

StokesMIDE commented 3 years ago

The new implementations of EventArray._getBlockIndexWithIndex() and EventArray_GetBlockIndexWithTime() are somewhat slower than the original. There's a comment in the code reading "profile & determine if this change is beneficial." Here are times using the old EventList methods (EventArray versions commented out):

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
  1241970    0.781    0.000    1.688    0.000 idelib\dataset.py:1417(EventArray._getBlockIndexWithTime)
        8    0.000    0.000    0.000    0.000 idelib\dataset.py:1400(EventArray._getBlockIndexWithIndex)

The EventArray version:

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
  1300631    4.031    0.000   17.953    0.000 idelib\dataset.py:2593(EventArray._getBlockIndexWithTime)
        8    0.000    0.000    0.016    0.002 idelib\dataset.py:2575(EventArray._getBlockIndexWithIndex)

(I did notice that the latter was called 8% more often, but the time differences are greater than that.)

This is probably because the EventArray version does a search of all blocks every time. The EventList version caches groups of times and indices, making the search smaller set to search.

StokesMIDE commented 3 years ago

Note: Removing the EventArray implementations caused some sporadic problems in the enDAQ Lab development branch (https://github.com/MideTechnology/SlamStickLab/issues/329). The method _getBlockIndexWithTime() was sometimes being called with a list of times rather than just one. My guess is the EventArray version simply isn't bombing in this case, which prevented the error that generated the list of times from being discovered.

End of a traceback in which the issue occurred:

  File "C:\Users\dstokes\workspace\SlamStickLab\venv\python39_endaq\lib\site-packages\idelib\dataset.py", line 2819, in iterSlice
    for times, values in self._blockSlice(start, end, step, display):
  File "C:\Users\dstokes\workspace\SlamStickLab\venv\python39_endaq\lib\site-packages\idelib\dataset.py", line 2798, in _blockSlice
    yield makeBlockEvents(
  File "C:\Users\dstokes\workspace\SlamStickLab\venv\python39_endaq\lib\site-packages\idelib\dataset.py", line 2528, in _makeBlockEvents
    times, values = retryUntilReturn(
  File "C:\Users\dstokes\workspace\SlamStickLab\venv\python39_endaq\lib\site-packages\idelib\dataset.py", line 2475, in retryUntilReturn
    value = func()
  File "C:\Users\dstokes\workspace\SlamStickLab\venv\python39_endaq\lib\site-packages\idelib\transforms.py", line 743, in __call__
    y = self._eventlist.getMeanNear(timestamp, outOfRange=True)
  File "C:\Users\dstokes\workspace\SlamStickLab\venv\python39_endaq\lib\site-packages\idelib\dataset.py", line 3216, in getMeanNear
    b = self._getBlockIndexWithTime(t)
  File "C:\Users\dstokes\workspace\SlamStickLab\venv\python39_endaq\lib\site-packages\idelib\dataset.py", line 1424, in _getBlockIndexWithTime
    blockIdx = bisect_right(self._blockTimes, t, start, stop)
ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()
StokesMIDE commented 3 years ago

The problem described in the previous comment may have been a result of the race condition described in issue #32 and fixed in PR #33.

StokesMIDE commented 3 years ago

Correction: the previous comment has not been resolved, it is just sporadic. For some reason, _getBlockIndexWithTime() is sometimes being called with a list of times rather than just one. The EventArray version doesn't fail, but it must be producing bad results.