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 apptainer files #10

Merged
merged 11 commits into from
Feb 4, 2025
Merged

Conversation

Edgar-21
Copy link
Contributor

No description provided.

Copy link
Member

@gonuke gonuke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Edgar-21 - this seems useful. I have a few suggestions and clarifying questions.

From: /software/chtc/containers/ubuntu/22.04/openmpi-4.1.6_gcc-11.3.0.sif

%arguments
HDF5_VERSION=hdf5_1_14_3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this should be defined in a way to be useful below? It's currently unused and the version is hard coded.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added args that allow controlling the hdf5 version by providing a url to wget from.

Comment on lines 44 to 45
libpng-dev \
libnetcdf-dev \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation

Comment on lines 50 to 51
wget https://support.hdfgroup.org/releases/hdf5/v1_14/v1_14_3/downloads/hdf5-1.14.3.tar.gz
tar -xvf hdf5-1.14.3.tar.gz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above comment on variables for this version

MOAB_VERSION=5.3.0
DOUBLE_DOWN_VERSION=v1.0.0
EMBREE_VERSION=v3.12.2
DAGMC_VERSION=v3.2.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not newer versions? v3.2.3 has been around a while and v3.2.4 just dropped

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try with the latest supported versions of moab, dagmc and double down and see, but this build process is fairly fragile.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

working with dagmc 3.2.4, moab 5.5.1, doubledown 1.1.0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and embree 4.3.3


apptainer build \
--bind $TMPDIR:/tmp \
"${@:2}" \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nothing, it has been removed

#SBATCH --error=job.%J.err
#SBATCH --output=job.%J.out

export JOB_TMP_PATH=/local/$USER/${SLURM_JOB_ID}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like the only line that is CHTC specific, because of the path, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I think so? I've not used other clusters not sure what might change

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can add a comment that notes that this particular path is correct for CHTC/HPC so that folks no where to change if they want to try this elsewhere

@@ -0,0 +1,148 @@
Bootstrap: localimage
From: /software/chtc/containers/ubuntu/22.04/openmpi-4.1.6_gcc-11.3.0.sif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does CHTC publish the def file for this image? This is the only line that makes this CHTC-specific, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the def file is available in that same directory. I think this image is designed to have the openmpi built in a way that is compatible, and somehow also interfaces with spack, to what end I am not sure.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just trying to think about how to make this portable to other systems, or at least note where others would need to be careful if trying to port to other systems. I wonder if it's possible to summarize what this image has in a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a few comments noting where things are uw hpc specific, and a brief explanation of the base image, as I understand it.

@Edgar-21 Edgar-21 requested a review from gonuke January 16, 2025 22:33
Copy link
Contributor

@lewisgross1296 lewisgross1296 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few comments and suggestions. Open to discuss if you don't like all my suggestions

Comment on lines 1 to 6
#!/bin/bash
#SBATCH --partition=pre # default "shared", if not specified
#SBATCH --time=0-24:00:00 # run time in days-hh:mm:ss
#SBATCH --nodes=10 # require 1 nodes
#SBATCH --ntasks-per-node=32 # cpus per node (by default, "ntasks"="cpus")
#SBATCH --mem=30000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to mention the upper limits for things like time, nodes, ntasks-per-node, and mem. I know these are probably on the CHTC website, but it would be nice to have in the script so you don't have to stop what you're doing to look it up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd prefer to avoid duplicating this information, like you mentioned its on the CHTC website. I'll remove the existing comments for consistency

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. Especially if these values change in the future, we'd want to make sure the info is accurate. Probably best just to direct people to the CHTC page. Can you add the link to the README.md in this repo?

Comment on lines 9 to 11

module load openmpi
# image_path is the path to the Apptainer .sif file you wish to use
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
module load openmpi
# image_path is the path to the Apptainer .sif file you wish to use
# load required modules here
module load openmpi
# image_path is the path to the Apptainer .def or .sif file you wish to use

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an example to submit a job, I don't think a .def is applicable here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can submit jobs to build containers :)

apptainer_files/example_submit_script.sh Outdated Show resolved Hide resolved
apptainer_files/example_submit_script.sh Outdated Show resolved Hide resolved
Comment on lines 2 to 6
#SBATCH --partition=pre
#SBATCH --time=0-04:00:00
#SBATCH --nodes=1
#SBATCH --ntasks-per-node=8
#SBATCH --mem-per-cpu=4000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we maybe request a bit more power to build this quicker?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call

@gonuke
Copy link
Member

gonuke commented Feb 4, 2025

I think @lewisgross1296 should merge when he decides this is ready

@Edgar-21
Copy link
Contributor Author

Edgar-21 commented Feb 4, 2025

Confirmed to be working on UW HPC.

@lewisgross1296 lewisgross1296 merged commit 0a1672c into cnerg:main Feb 4, 2025
@Edgar-21 Edgar-21 deleted the add_apptainer_files branch February 4, 2025 20:57
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