Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the Kilosort integration to run in a separate conda environment instead of requiring Kilosort to be installed in the main bnd environment. The main code executes Kilosort by spawning a subprocess via conda run -n kilosort, passing parameters through a temporary JSON file.
Changes:
- Removed direct imports of
torchandkilosortfrom the main module - Added subprocess-based runner that executes Kilosort in a separate conda environment (configurable via
BND_KILOSORT_ENV) - Updated README with instructions for installing Kilosort in a separate environment
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| bnd/pipeline/kilosort.py | Refactored to run Kilosort via subprocess in separate conda env; added helper functions for env detection, conda runner discovery, CUDA checking, and subprocess execution |
| README.md | Updated installation instructions to document the separate Kilosort environment setup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bnd/pipeline/kilosort.py
Outdated
| def _check_kilosort_cuda(env_name: str) -> tuple[bool, str | None]: | ||
| code = textwrap.dedent( | ||
| """ | ||
| import torch | ||
|
|
||
| if torch.cuda.is_available(): | ||
| print("CUDA_AVAILABLE") | ||
| print(torch.cuda.get_device_name(0)) | ||
| else: | ||
| print("CUDA_NOT_AVAILABLE") | ||
| """ | ||
| ).strip() | ||
|
|
||
| proc = _run_in_conda_env(env_name, ["python", "-c", code], capture_output=True) |
There was a problem hiding this comment.
If torch is not installed in the kilosort environment, this code will fail with an import error, but the error message from _run_in_conda_env will be generic. Consider catching import errors specifically and providing a more helpful error message that mentions torch needs to be installed in the kilosort environment per the README instructions.
worked on 1 test session.
@martinesparza or @ioanalzr can you test it on one of yours?