Skip to content

Support apptainer instead of singularity#152

Merged
ffelten merged 3 commits into
mainfrom
feat/support_apptainer
Jul 24, 2025
Merged

Support apptainer instead of singularity#152
ffelten merged 3 commits into
mainfrom
feat/support_apptainer

Conversation

@ffelten
Copy link
Copy Markdown
Collaborator

@ffelten ffelten commented Jul 24, 2025

I'm not sure how to test this though?

@ffelten ffelten requested a review from g-braeunlich July 24, 2025 07:03
@g-braeunlich
Copy link
Copy Markdown
Collaborator

@ffelten In hpc-zaratan there are some conflicting changes with your branch.
Could we merge his branch first?

Instead of the changes of the commit Support apptainer instead of singularity, I would propose

class Apptainer(Singularity):
    """Apptainer."""

    name = "apptainer"
    executable = "apptainer"

So we support both apptainer and singularity.
Already have this in my working tree.

@g-braeunlich
Copy link
Copy Markdown
Collaborator

g-braeunlich commented Jul 24, 2025

@ffelten May I cherry-pick 6a8339e to this branch, replacing Use apptainer instead of singularity?

@ffelten
Copy link
Copy Markdown
Collaborator Author

ffelten commented Jul 24, 2025

@g-braeunlich sure! Good idea

@ffelten
Copy link
Copy Markdown
Collaborator Author

ffelten commented Jul 24, 2025

Note: my student just confirmed that this new version does not have clashes relating to MPI (from different jobs) writing in the same tmp folder.

@g-braeunlich
Copy link
Copy Markdown
Collaborator

g-braeunlich commented Jul 24, 2025

@ffelten I cherry-picked and rewrote the history a bit (squashed your commits, changed the commit message) into a new branch: support_apptainer-rebased.
To view effective changes:

git diff origin/feat/support_apptainer origin/feat/support_apptainer-rebased

@g-braeunlich
Copy link
Copy Markdown
Collaborator

After my changes, my only suggestion would be to use tempfile.TemporaryDirectory instead of /tmp (increase portability)

@ffelten
Copy link
Copy Markdown
Collaborator Author

ffelten commented Jul 24, 2025

I guess we should close this one?

@g-braeunlich
Copy link
Copy Markdown
Collaborator

The new branch was actually meant as a proposal to replace this branch, e.g. locally, on feat/support_apptainer:

git fetch
git reset --hard origin/feat/support_apptainer-rebased
git push -f

@ffelten ffelten force-pushed the feat/support_apptainer branch from 98d091d to 9df3fee Compare July 24, 2025 10:37
@ffelten
Copy link
Copy Markdown
Collaborator Author

ffelten commented Jul 24, 2025

@g-braeunlich done!

Actually, I'm not sure we want to keep supporting singularity; Euler and Zaratan support Apptainer. Moreover, the last commit on the repo was 3 years ago.

@ffelten ffelten mentioned this pull request Jul 24, 2025
@g-braeunlich g-braeunlich force-pushed the feat/support_apptainer branch from 9df3fee to a7af697 Compare July 24, 2025 12:11
@g-braeunlich
Copy link
Copy Markdown
Collaborator

OK. I reverted to apptainer only

@g-braeunlich
Copy link
Copy Markdown
Collaborator

After my changes, my only suggestion would be to use tempfile.TemporaryDirectory instead of /tmp (increase portability)

And did you see this?

@ffelten
Copy link
Copy Markdown
Collaborator Author

ffelten commented Jul 24, 2025

And did you see this?

Missed it. We can use it indeed. I see you're on the branch. Do you want me to do it or you do it?

@g-braeunlich
Copy link
Copy Markdown
Collaborator

OK. I ll change that

@g-braeunlich g-braeunlich force-pushed the feat/support_apptainer branch from a7af697 to 5f72e29 Compare July 24, 2025 12:46
@g-braeunlich
Copy link
Copy Markdown
Collaborator

Changed. I not went for tempfile.gettempdir(), as it does not contain an unpredictable directory and does not include directory cleanup.

@g-braeunlich g-braeunlich force-pushed the feat/support_apptainer branch from 5f72e29 to d8dae9d Compare July 24, 2025 13:00
@g-braeunlich
Copy link
Copy Markdown
Collaborator

Rebased on main (after the multi os PR)

@ffelten ffelten merged commit 1fe2966 into main Jul 24, 2025
10 checks passed
@ffelten ffelten deleted the feat/support_apptainer branch July 24, 2025 15:04
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