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.

Sign up for GitHub

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

Jump to bottom

Update for Sherlock #256

Merged
merged 8 commits into from
Feb 21, 2024
Merged

Update for Sherlock #256

merged 8 commits into from
Feb 21, 2024

Conversation

matteofrigo5
Copy link
Contributor

@matteofrigo5 matteofrigo5 commented Jan 26, 2024 •
edited
Loading

Main changes:

  1. Change the INSTALL_DIR_ROOT
  2. Change the DOCKER_ROOT_IMAGE (update to CUDA 11.7.1)

Relates to GEOS-DEV/GEOS#2977

CusiniM
CusiniM reviewed Jan 27, 2024
View reviewed changes
.github/workflows/docker_build_tpls.yml Outdated
@@ -48,7 +48,7 @@ jobs:
DOCKERFILE: docker/Stanford/Dockerfile
DOCKER_ROOT_IMAGE: jafranc/sherlock-gcc10.1.0-openmpi4.1.2-cuda11.5.0-openblas0.3.10-zlib1.2.11-no-geosx:0.0.1
HOST_CONFIG: docker/Stanford/sherlock-gcc10-ompi4.1.2-openblas0.3.10-cuda11.5.0-sm80.cmake
INSTALL_DIR_ROOT: /oak/stanford/schools/ees/COLLABORATIONS/geosx/GPU
INSTALL_DIR_ROOT: /home/groups/tchelepi/geosx-sherlock/GPU
Copy link
Contributor

Choose a reason for hiding this comment

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

why not just geos ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, put geos whenever you can.

@TotoGaz TotoGaz mentioned this pull request Feb 2, 2024
Merged
TotoGaz
TotoGaz approved these changes Feb 2, 2024
View reviewed changes
Copy link
Contributor

@TotoGaz TotoGaz left a comment

Choose a reason for hiding this comment

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

Please update the title and the description of the PR.

@matteofrigo5 matteofrigo5 changed the title Update INSTALL_DIR_ROOT for Sherlock Update for Sherlock Feb 2, 2024
@TotoGaz TotoGaz marked this pull request as ready for review February 3, 2024 00:06
@jafranc
Copy link
Contributor

jafranc commented Feb 6, 2024 •
edited
Loading

@matteofrigo5 Are we sure on the final path it will be in ? Or will it be in /oak/stanford/groups/tchelepi ?

@matteofrigo5
Copy link
Contributor Author

I've create two distinct directories, namely /oak/stanford/groups/tchelepi/geos-sherlock/CPU and /oak/stanford/groups/tchelepi/geos-sherlock/GPU, to segregate the binaries for the respective deployments of CPU and GPU.
@jafranc do you think it might create some problems?

@jafranc
Copy link
Contributor

jafranc commented Feb 6, 2024 •
edited by matteofrigo5
Loading

I've create two distinct directories, namely /oak/stanford/groups/tchelepi/geos-sherlock/CPU and /oak/stanford/groups/tchelepi/geos-sherlock/GPU, to segregate the binaries for the respective deployments of CPU and GPU. @jafranc do you think it might create some problems?

No that's good, then this PR should reflect those choices :)
And you're good to merge and deploy

The PR reflect those choices:

thirdPartyLibs/.github/workflows/docker_build_tpls.yml

Line 45 in 23c1499

INSTALL_DIR_ROOT: /oak/stanford/groups/tchelepi/geos-sherlock/CPU

thirdPartyLibs/.github/workflows/docker_build_tpls.yml

Line 51 in 23c1499

INSTALL_DIR_ROOT: /oak/stanford/groups/tchelepi/geos-sherlock/GPU

Do I need to edit any other file?

TotoGaz
TotoGaz reviewed Feb 6, 2024
View reviewed changes
docker/Stanford/Sherlock.Dockerfile Outdated
@@ -182,6 +182,6 @@ ENV CC=${SHERLOCK_GCC_INSTALL_DIR}/bin/gcc \
MPIFC=${SHERLOCK_OPENMPI_INSTALL_DIR}/bin/mpifort \
MPIEXEC=${SHERLOCK_OPENMPI_INSTALL_DIR}/bin/mpiexec \
# An additional `LD_LIBRARY_PATH` action is needed for the tools to work.
LD_LIBRARY_PATH=${SHERLOCK_OPENMPI_INSTALL_DIR}/lib:${SHERLOCK_GCC_INSTALL_DIR}/lib64:${SHERLOCK_OPENBLAS_INSTALL_DIR}/lib:${SHERLOCK_ZLIB_INSTALL_DIR}/lib:${LD_LIBRARY_PATH}
LD_LIBRARY_PATH=${SHERLOCK_OPENMPI_INSTALL_DIR}/lib:${SHERLOCK_GCC_INSTALL_DIR}/lib64:${SHERLOCK_OPENBLAS_INSTALL_DIR}/lib:${SHERLOCK_CUDA_INSTALL_DIR}/lib64:${SHERLOCK_ZLIB_INSTALL_DIR}/lib:${LD_ LIBRARY_PATH}
Copy link
Contributor

Choose a reason for hiding this comment

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

😨 fix those spaces!

@TotoGaz
Copy link
Contributor

TotoGaz commented Feb 6, 2024

Do I need to edit any other file?

I'd say it's OK!
Now it's possible (and probably a good idea 🤣 ) to test before merging!

@jafranc
Copy link
Contributor

jafranc commented Feb 6, 2024

I've create two distinct directories, namely /oak/stanford/groups/tchelepi/geos-sherlock/CPU and /oak/stanford/groups/tchelepi/geos-sherlock/GPU, to segregate the binaries for the respective deployments of CPU and GPU. @jafranc do you think it might create some problems?

No that's good, then this PR should reflect those choices :) And you're good to merge and deploy

The PR reflect those choices:

thirdPartyLibs/.github/workflows/docker_build_tpls.yml

Line 45 in 23c1499

INSTALL_DIR_ROOT: /oak/stanford/groups/tchelepi/geos-sherlock/CPU

thirdPartyLibs/.github/workflows/docker_build_tpls.yml

Line 51 in 23c1499

INSTALL_DIR_ROOT: /oak/stanford/groups/tchelepi/geos-sherlock/GPU

Do I need to edit any other file?

Not that I recall, @TotoGaz is it the only location even post-GA introduction ?

@TotoGaz
Copy link
Contributor

TotoGaz commented Feb 6, 2024

is it the only location even post-GA introduction ?

The TPLs and geos will be installed in this folder.

@TotoGaz TotoGaz merged commit a2fe862 into master Feb 21, 2024
12 checks passed
@TotoGaz TotoGaz deleted the update-sherlock branch February 21, 2024 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TotoGaz TotoGaz TotoGaz approved these changes

@CusiniM CusiniM CusiniM approved these changes

@jafranc jafranc Awaiting requested review from jafranc

Assignees

@matteofrigo5 matteofrigo5

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@matteofrigo5 @jafranc @TotoGaz @CusiniM