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

First attempt at "Automatic Image generation and deployment" #76

Closed
wants to merge 1 commit into from

Conversation

tabedzki
Copy link

@tabedzki tabedzki commented Jun 3, 2024

Hi there,

I wanted to propose a starting point to address Issue #75 about automatic image generation.

I have written a script that looks at the Kilosort's github page's releases and takes the most recent v4 version. It overwrites the Dockerfile to have the latest version of kilosort4 and then attempt to build a docker image.

I have tested the python script locally but have not tested the action itself. I think this servers as a good starting point for a discussion about how to maintain up-to-date images for the fastest updating spikesorting package.

@alejoe91
Copy link
Member

alejoe91 commented Jun 3, 2024

Hi @tabedzki

Thanks for the contribution. Kilosort4 is changing quite fast, so every new release needs a proper test in SpikeInterface. For example, v4.0.7/4.0.8 are currently not supported, because of SpikeInterface/spikeinterface#2839

So I don't think that currently it's a good idea to automatically build and push new images as they are released.

@tabedzki
Copy link
Author

tabedzki commented Jun 3, 2024

@alejoe91 ahh okay, thanks

  1. Is the general thinking that SpikeInterface will be releasing known compatible images that people should use?
  2. Since we know that spikeinterface is incompatible with certain versions of kilosort, should we have a check to ensure that when one installs spikeinterface via pip that incompatible versions of spikesorters are not installed?
    • I didn't specify versions (python -mpip install kilosort 'spikeinterface[full]') and pip installed both spikeinterface at 100.0.6 and kilosort4 at 4.0.8 which people might know are incompatible

@JoeZiminski
Copy link

Thanks @tabedzki this looks really cool. I did not realise the point by @alejoe91 that some kilosort releases will break SpikeInterface compatability and so it cannot be fully automated. It would be nice to have as-automated-as-possible release cycle because I think relying on manually handling new versions is a lot of work and possibly not sustainable in the long-term. The release cycle is too fast ATM and this is leading to missing versions on dockerhub. Maybe the workflow could be as below, forunately testing is easier as kilosort4 can run on CPU / in python.

  1. The .github/workflows/checking_kilosort4_releases.yml runs to check kilosort4 releases. It also runs a test to check running kilosort4 in spikeinterface vs. running it natively returns identical outputs. It can pull the pypi kilosort, the recent spikeinterface main branch, and run kilosort4 through and not through spikeinterface on a small test file. This can parameterise over all arguments to check the signatures have not changed.
  2. This will either fail (requires looking into) or pass. It could be setup to send out emails or some other notificaton system.
  3. If failing or passing, a PR can be opened like 'updating to kilosort 4.0.10. This can focus on making and discussing any required changes. It will probably still be required to check GPU outputs running either manually or on local runners. On agreement, the build section of the CI can be triggered, either by some specific commit name, or by setting it up to trigger on PR merge with specific name, or something like that.

I am quite keen on building some local runners to fully test all kilosort versions in this way, but it is a pain because of the GPU, so am happy to get started on KS4 which is low-hanging-fruit if people agree it is a good approach.

@JoeZiminski
Copy link

Since we know that spikeinterface is incompatible with certain versions of kilosort, should we have a check to ensure that when one installs spikeinterface via pip that incompatible versions of spikesorters are not installed?
I didn't specify versions (python -mpip install kilosort 'spikeinterface[full]') and pip installed both spikeinterface at 100.0.6 and kilosort4 at 4.0.8 which people might know are incompatible

This is a great point, I have had users indicate they cannot get KS4 working with SpikeInterface and I think this is the issue. Something like checking the KS4 versions that have dockerfiles, to build a list of 'supported' KS4 versions, and throwing at error if the local version is not supported, could be an approach. Of course it is also possible to manually maintain a list but the scope for error e.g. forgetting to update it would be larger.

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

Successfully merging this pull request may close these issues.

3 participants