Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
StokesMIDE opened this issue Feb 10, 2021 · 3 comments
Open

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

StokesMIDE opened this issue Feb 10, 2021 · 3 comments
Labels
enhancement New feature or request

Comments

@StokesMIDE
Copy link
Member

StokesMIDE commented Feb 10, 2021

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 StokesMIDE added the enhancement New feature or request label Feb 10, 2021
@StokesMIDE
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant