Minor Issues with KiloSort2 data
Closed this issue · 5 comments
Just started using phy with the output of KS2, which does some handy things compared to KS1, like computing template amplitudes, contamination rates, and estimates an labels whether a unit is good or mua.
All of these stats show up in phy, but there are some bugs it seems, the biggest one being that sometimes if you split or merge, or undo a split or merge, the phy gui will wipe all of the KS2 labels and other statistics for all units (ie. turns them all to unsorted - except manually labelled clusters, and nulls all template amplitudes). While not fatal, it is a pain...
This issue will persist if you close and reopen phy, and even persists after deleting the log and .phy folder. My guess is that this has something to do with a mismatch between the labels in the KS_labels.tsv file and cluster indices that are being updated in spike_templates.npy.
A possibly related issue is that when previously sorting KS1 data, the phy gui would automatically save data, whether ctrl-s is pressed or not, such that when you reopen the GUI you can continue from where you left off. It seems that this is not the case with KS2 data.
Great point. @rossant it should be the default behavior that if a csv file has a row corresponding to a cluster ID that doesn't exist, then that row is ignored. And, if there is a cluster ID that doesn't match any row in the csv file, it should just get null. We could think about better ways to deal with this that are a bit more complicated, but that should be a quick fix as this issue is urgent (we would like people to be using phy to check KS2 results now; also, I disagree with the assessment above: this is fatal, w/r/t the csv file functionality). Would it be quick to fix?
At the same time this other issue could be fixed, that an incorrectly-formatted csv file present in the directory causes phy to crash with an uninformative error message (should get "warning: could not parse 'mydumbfile.csv'").
I would just put a try/except clause here :
phy-contrib/phycontrib/template/model.py
Lines 317 to 318 in 1f02da4
something like:
try:
field_name, values = load_metadata(filename)
metadata[field_name] = values
except Exception as e:
logger.warning("Could not parse %s", filename)
Could you try to see if it fixes the problem, or if there's a need for a more sophisticated fix?
@angelonc could you give that a try?
I made rossant's suggested edits to model.py, but still confirm angelonc's finding about null values appearing for newly split units. I forget the exact order, but doing the usual sequential split, undo-operations, at one point, all units appeared to have their KS values turned to "null" including amplitudes, contamination rates, labels, etc... Thankfully auto-save doesn't appear to be active, so closing phy and restarting at least is able to undo this flaw and restart from an earlier save. Agree that this is a critical bug.
should be fixed in the dev branch of phy!