-
Notifications
You must be signed in to change notification settings - Fork 1
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
Implement alternative bout detection method #15
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice addition to the repo. If it works for you I see no reason to not merge it. I just added a couple comments so it can be useful (and not confusing) for other people to use.
array of correlation scores that matches with the trace by index. | ||
""" | ||
|
||
if kernel is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An issue I see with this approach is that the kernel is probably quite dependent on the rate at what your data has been sampled. One could up/downsample this kernel, but then it should at least be indicated the sampling rate of the data it is based on, and instead of hardcoded in the function, maybe it could be stored in an assets/kernels/whatever folder from where one can load it and modify it before using it, rather than just having to alter it within the function.
Maybe (big maybe) adding a function/instruction on how to generate one's own?
"""Extracts all bouts from a freely-swimming tracking experiment | ||
|
||
:param exp: the experiment object | ||
:param scale: mm per pixel, recalculated by default | ||
:param threshold: velocity threshold in mm/s | ||
:param threshold: velocity threshold in mm/s or score threshold if conv_detection=True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if the fact that this threshold is provided via the same argument can lead to small issues with people not realizing the thresholds for both methods are in different units, and therefore not detecting bouts properly.
One could split this into two different threshold arguments, but maybe it makes the function uglier with lots of arguments. Alternatively, maybe adding a warning if your method is used, pointing towards thinking about the provided threshold?
relative_values = current_values / (np.max(current_values) + eta) | ||
|
||
# Simplified convolution, works faster and with Numba. | ||
conv = np.sum(relative_values * np.flip(kernel)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only issue with using convolutions is one needs to be aware that, depending on the threshold used, bouts may end up being detected earlier than they occurred. If possible, I would try to make clear somewhere in the docs that one needs to keep this in mind when processing and analyzing the data.
I changed the scope of this PR. Originally it was meant to fix a specific mechanism in the bout detection algorithm. Over time I realised this needed an additional number of mechanisms to make things work correctly.
Instead I'd like to add an alternative bout detection method in the hope that it'll be useful to others. The basic idea is that it convolves the squared velocity trace with a kernel which results in a trace with peaks. The peaks indicate the bouts and the slopes can be used for estimating the start and end of a bout.
The kernel is hard-coded. It is the result of fitting data from my experiments in a neural network consisting of a 100 weights 1D convolution kernel and a sigmoid.