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

Add singularity compatibility #1608 #1729

Merged
merged 2 commits into from
Aug 16, 2023
Merged

Add singularity compatibility #1608 #1729

merged 2 commits into from
Aug 16, 2023

Conversation

gwenzek
Copy link
Contributor

@gwenzek gwenzek commented Mar 2, 2023

This is a long awaited feature request #1608
the feature has been tested for some time.

This adds a "python" variable to "SlurmExecutor".
SlurmExecutor will use this given python executable/command to launch job.
The user is responsible to escape the string, which allows to pass singularity command like singularity --nv exec my_image.sif /opt/python3.8/bin/python

In AutoExecutor you can use AutoExecutor(slurm_python="singularity --nv exec my_image.sif /opt/python3.8/bin/python")

@gwenzek gwenzek requested a review from jrapin March 2, 2023 13:50
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 2, 2023
@srossi93
Copy link

Hi everyone, would it be possible to merge this PR? Is there a particular blocking point preventing the merge? I would be happy to contribute, if this needs more work.

@jrapin
Copy link
Contributor

jrapin commented Aug 16, 2023

@gwenzek can this be moved to the base executor class? I expect the same logic can be applied to LocalExecutor (but not DebugExecutor) and any other executor based on processes

Merging anyway since this version has been version for some time now, this can unblock people, moving to the base Executor should not be a breaking change, and you're not available right now ;)

@jrapin jrapin merged commit f834db5 into main Aug 16, 2023
@jrapin jrapin deleted the escape_all branch August 16, 2023 07:28
@MaximilienLC
Copy link

MaximilienLC commented Oct 20, 2023

Hey @jrapin @gwenzek! Any update on moving this to the base executor class? Thanks a lot :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants