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

Update Cellpose from 2.2.2 to 3.0.1 #559

Merged
merged 5 commits into from
Feb 22, 2024

Conversation

FloWuenne
Copy link
Contributor

Submitting a Container

(If you're requesting for a new container, please check the procedure described here.

Check BioContainers' Dockerfile specifications

Checklist

  1. Misc
  • My tool doesn't exist in BioConda
  • The image can be built
  1. Metadata
  • LABEL base_image
  • LABEL version
  • LABEL software.version
  • LABEL about.summary
  • LABEL about.home
  • LABEL about.license
  • MAINTAINER
  1. Extra (optionals)
  • I have written tests in test-cmds.txt
  • LABEL extra.identifier
  • LABEL about.documentation
  • LABEL about.license_file
  • LABEL about.tags

Check BioContainers' Dockerfile metadata

@biocontainers-bot
Copy link
Collaborator

No test-cmds.txt (test file) present, skipping tests

@FloWuenne
Copy link
Contributor Author

This PR adds the newest version of Cellpose, some of the changes:

  • python 3.11 as base image instead of 3.9
  • Added new MPLCONFIGDIR and NUMBA_CACHE_DIR to circumvent issues when running container in nextflow

Currently, there is a chmod 777 command for permissions to folder ./cellpose. If possible, would like to remove this, but I don't know how to specify this differently so that permissions for that folder for writing are still given.

@FloWuenne FloWuenne changed the title Cellpose 2.3.2 Update Cellpose from 2.2.2 to 2.3.2 Feb 16, 2024
@mboudet
Copy link
Contributor

mboudet commented Feb 16, 2024

Hello! Thanks for the PR.

Not sure what do you mean regarding writing permissions. At which step are the permissions required?
It would be great to merge the pip install commands into one RUN if possible (either directly, or by using a requirement file).

@FloWuenne
Copy link
Contributor Author

Thanks for the comments @mboudet!

I was referring to this line: https://github.com/FloWuenne/containers/blob/12a2df77f99ec7e77cd6edb8fa0399f0d209d93a/cellpose/2.3.2/Dockerfile#L28

We were having issues with permissions when running cellpose from within the container when not making the ./cellpose directory and giving it 777 permissions. Generally its quite bad practice to use chmod 777 in containers and so I was wondering whether there is another way we could try to have this folder have the right permissions without explicitly calling chmod 777.

@FloWuenne
Copy link
Contributor Author

@mboudet I have merged the pip commands into one Layer and have changed the permission by adding a user, giving that user permissions and running the container as that user. This was suggested by ChatGPT but I have seen this approach suggested in some forums too. Is this an acceptable approach?

@biocontainers-bot
Copy link
Collaborator

No test-cmds.txt (test file) present, skipping tests

@biocontainers-bot
Copy link
Collaborator

No test-cmds.txt (test file) present, skipping tests

@mboudet
Copy link
Contributor

mboudet commented Feb 19, 2024

Thanks for the changes regarding pip!

I'm guessing the permission issue is for Singularity? (Given that the docker is run as root, it should not matter for a docker run)
(Does it even works in Singularity? I though images where non-writable..)

@FloWuenne
Copy link
Contributor Author

@mboudet Actually the permission issue is not for singularity but is affecting docker when using the container with Nextflow.

I was unable to make the container run in Nextflow with any setting beside forcing the user to be root in Nextflow. I don't quite understand why the permissions within nextflow need to be root for this specific container and the folders it creates when downloading models...

I will check out the new release 3.0.0 and if that fixes these issues we might just skip this one and go directly for 3.0.0!

@mboudet
Copy link
Contributor

mboudet commented Feb 21, 2024

Oh, interesting. I don't usually run nextflow with docker containers (only singularity), so I'm not sure what is happening permission-wise.
I see that cellpose needs to download some model files, so I guess if nextflow use another user than root it will not have permissions.
You might try to mount the /.cellpose folder from outside the docker maybe? (Might be for the best anyway, that would avoid redownloading the files on each run).

In any case, I don't think the chmod 777 is an issue. You have it set to 755 currently though, so it will only work if it needs read permission (and not write).

Alright for the release 3.0.0, you can juste update this PR with the correct release when it's ready.

@FloWuenne
Copy link
Contributor Author

FloWuenne commented Feb 21, 2024

@mboudet I have updated to version 3.0.1 after testing that it works with old trained models and within our nextflow setup.

I have also figured out the issue with permission. We needed to add a new variable:
ENV CELLPOSE_LOCAL_MODELS_PATH=/tmp/cellpose_models

This moves the models that are downloaded out of $HOME and thus removes the permissions issues. This image should now be good for version 3.0.1!

@FloWuenne FloWuenne changed the title Update Cellpose from 2.2.2 to 2.3.2 Update Cellpose from 2.2.2 to 3.0.1 Feb 21, 2024
@biocontainers-bot
Copy link
Collaborator

No test-cmds.txt (test file) present, skipping tests

@mboudet
Copy link
Contributor

mboudet commented Feb 22, 2024

Perfect, thank you!

@mboudet mboudet merged commit 8dc63e1 into BioContainers:master Feb 22, 2024
1 check passed
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