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

Avoid segmentation error during kilosort2 cluster splitting #987

Closed
TomBugnon opened this issue Oct 6, 2022 · 5 comments
Closed

Avoid segmentation error during kilosort2 cluster splitting #987

TomBugnon opened this issue Oct 6, 2022 · 5 comments
Labels
question General question regarding SI

Comments

@TomBugnon
Copy link
Contributor

Hi
We have been experiencing segmentation errors during cluster splitting with kilosort2.5
We resolved this by installing a different version of the mkl library and, in our fork, setting the corresponding (system specific) paths as environment variables in kilosort_master.m as follows:

setenv('BLAS_VERSION', '/usr/lib/x86_64-linux-gnu/mkl/libblas.so');
setenv('LAPACK_VERSION', '/usr/lib/x86_64-linux-gnu/mkl/liblapack.so');

Allowing this in df spikeinterface is pretty much the last step we need before we can get rid of our fork
Do you think that would be possible? Do you have a preference about how to do this? We could for instance add the paths to BLAS and LAPACK as kilosort parameters.

We'll be happy to submit a PR implementing whichever approach seems preferable.
Best
Tom

@alejoe91
Copy link
Member

alejoe91 commented Oct 6, 2022

Hi @TomBugnon

Would setting them as env variables solve your issue, as suggested by @Dradeliomecus here?

If that is not propagated to MATLAB, then we could check in the KS wrappers if the env variable is set and add the setenv to the matlab script accordingly.

@samuelgarcia
Copy link
Member

@alejoe91 : Maybe it would be more convinient to add some option in kilosort with theses env variable that could injected in the generated script as suggested.
If we use the slurm laucher then the solution of @Dradeliomecus will not work but if it is inside the shell script then it should be OK.

@alejoe91
Copy link
Member

alejoe91 commented Oct 6, 2022

Ok good for me!
@TomBugnon could you make a PR? Adding a blas_path and lapack_path parameters as parameters (with None as default) should work

@TomBugnon
Copy link
Contributor Author

TomBugnon commented Oct 11, 2022

I prefer also the approach of setting those as parameters (so that different users on the same system can expect the same results when using the same spikeinterface parameters). Will submit a PR asap.

@alejoe91 alejoe91 added the question General question regarding SI label Oct 13, 2022
@alejoe91
Copy link
Member

Fixed by #893

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question General question regarding SI
Projects
None yet
Development

No branches or pull requests

3 participants