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

Incorrect 'parent_recording' arg for multiprocessing when skipping kilosort 2.5 preprocessing #3617

Open
OlivierPeron opened this issue Jan 14, 2025 · 16 comments
Labels
bug Something isn't working

Comments

@OlivierPeron
Copy link
Contributor

Hi everyone !

I was trying to launch kilosort 2.5 with skip_kilosort_preprocessing = True, which will use the TracePaddedRecording class, which have an parent_recording key in the __init__ part.
I got the following error :

File "<string>", line 1, in <module>
  File "C:\Users\DeepLabCut\Anaconda3\envs\spykeline\lib\multiprocessing\spawn.py", line 116, in spawn_main
    exitcode = _main(fd, parent_sentinel)
  File "C:\Users\DeepLabCut\Anaconda3\envs\spykeline\lib\multiprocessing\spawn.py", line 126, in _main
    self = reduction.pickle.load(from_parent)
  File "C:\Users\DeepLabCut\Anaconda3\envs\spykeline\lib\site-packages\spikeinterface\core\base.py", line 529, in from_dict
    extractor = _load_extractor_from_dict(dictionary)
  File "C:\Users\DeepLabCut\Anaconda3\envs\spykeline\lib\site-packages\spikeinterface\core\base.py", line 1135, in _load_extractor_from_dict
    extractor = extractor_class(**new_kwargs)
TypeError: __init__() got an unexpected keyword argument 'parent_recording'

Is it something wrong on my side or should we change the parent_recording key?

Thanks !!

@zm711
Copy link
Collaborator

zm711 commented Jan 14, 2025

Which version of spikeinterface are you using? We just want to make sure this is a current issue :) I think we got everything changed over.

@OlivierPeron
Copy link
Contributor Author

I am using the version 0.101.2.

@zm711
Copy link
Collaborator

zm711 commented Jan 14, 2025

Was the dataset old? Once I know that then I can start digging. I'm wonder if we write out the json wrong. But I want to confirm that this isn't some old dataset that we are just reloading with an old name.

@OlivierPeron
Copy link
Contributor Author

The dataset is like 4 yo, and it was acquired with intan if that helps.

@zm711
Copy link
Collaborator

zm711 commented Jan 14, 2025

Could you try loading the file after transferring just the intan file to a new folder. So we make sure there isn't any cache of json files that might be providing the wrong name. If the error still occurs in a clean folder with just the intan file then it's a bug for us. If that works then it means an old file was confusing things.

@OlivierPeron
Copy link
Contributor Author

Here is the beginning of the json file :

image

and I mistake earlier but I am using version 0.101.1 on this computer. Could that be the issue ?

@zm711
Copy link
Collaborator

zm711 commented Jan 14, 2025

It is always worth trying the most recent (because I can't remember when we switched form parent_recording to just recording. But I would try my other thing first and see what happens.

@alejoe91
Copy link
Member

If the problem is parent_* we could have an automatic fix at the load level!

@OlivierPeron
Copy link
Contributor Author

I moved the .dat file to a new empty folder, but still got the same error...

@zm711
Copy link
Collaborator

zm711 commented Jan 14, 2025

Up to you @alejoe91 a global fix is probably safer just in case we do have older files floating around or other preprocessing with the old naming. I could try to track down where we are incorrectly writing the json if you think that would be better.

Thanks @OlivierPeron we will work it on shortly :)

@zm711 zm711 added the bug Something isn't working label Jan 14, 2025
@alejoe91
Copy link
Member

@zm711 I think we still have parent_recording and parent_sorting in the segment utils!!!

@alejoe91
Copy link
Member

alejoe91 commented Jan 14, 2025

and in a few preprocessors too!

So my suggestion is we fix them and then have something like this when loading JSON files:

json_txt = json_file.read_text()
json_txt = json_txt.replace("parent_recording", "recording")
json_txt = json_txt.replace("parent_sorting", "sorting")
extractor_dict = json.loads(json_txt)

@zm711
Copy link
Collaborator

zm711 commented Jan 14, 2025

I thought we left those for backward compatability because we thought they wouldn't cause issues. But clearly that is not the case. I know we discussed which ones to changes and which to leave the same at Edinburgh, but it was too long ago now.

@alejoe91
Copy link
Member

Ah, I remember we changed all of them! But then I'm not sure why this is triggereing an error...

@zm711
Copy link
Collaborator

zm711 commented Jan 14, 2025

I thought the parent was because for certain preprocessors we used it as provenance and didn't actually expose it to the other code. So parent in a more parent-child node system and that we tried to prevent its use at the api. But I guess we could change them and see if tests hold up.

@zm711
Copy link
Collaborator

zm711 commented Jan 14, 2025

Also I'm at a seminar thing for the next couple days, so I won't be able to fix this at the moment (+/- this weekend if no one else gets to it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants