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

Introduced user configurable memory specification to qsub #346

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

wdeshazer
Copy link
Contributor

Introduced User Configurable Memory Specification to gacode/shared/bin/gacode_qsub

  • Previously it was hardwired in Site Supplemental files in gacode/platform/qsub

Provisioned for -mem and -mem-per-cpu

Applied Precedence rules

  • Default -mem=16GB
  • Double specification by user
    • Warning
    • Default to user specified `-mem1
    • Otherwise use use specified values

Communicates select through environment variables

  • MEMPERNODE
  • MEMPERCPU
  • Logic nullifies other variable

Site Supplemental file responds correspondingly

  • Modified gacode/shared/bin/gacode_qsub
  • Modified Site Files
    • gacode/platform/qsub/qsub.PPPL
    • gacode/platform/qsub/qsub.OMEGA
  • Ran regression tests
    • Write regression tests to confirm behavior
  • Assess if gacode/shared/bin/gacode_qsub_multi needs same medicine
  • Peruse the remaining files in gacode/shared/bin to ensure compatibility
  • Peruse the remaining Site Files in gacode/platform/qsub to ensure compatibility

I have taken a cursory look at all files and don't see anything glaring, but it deserves more consideration

@wdeshazer wdeshazer requested review from smithsp and jmcclena February 6, 2024 12:48
@wdeshazer
Copy link
Contributor Author

wdeshazer commented Feb 6, 2024

Checklist of Confirmed Compatible files

gacode/shared/bin

AZURE-COMET
  • qsub.AZURE
  • qsub.AZURE_GPU
  • qsub.AZURE_HB2
  • qsub.AZURE_HB2_1k
  • qsub.AZURE_HC
  • qsub.AZURE_SLURM
  • qsub.BANACH
  • qsub.CLOUD_GPU
  • qsub.COMET
  • qsub.CRUSHER
DAINT_PGI-HPC_ITER
  • qsub.DAINT_PGI
  • qsub.DROP
  • qsub.EDISON_CRAY
  • qsub.EDISON_IFORT
  • qsub.FREIA
  • qsub.Frontera_GCC
  • qsub.Frontera_IFORT
  • qsub.FRONTIER
  • qsub.GASUMMIT_GPU
  • qsub.HPC_ITER
IRIS-OMEGA
  • qsub.IRIS
  • qsub.KAIROS
  • qsub.LOKI
  • qsub.MARCONI
  • qsub.MARCONI_KNL
  • qsub.MARCONI_LEONARDO
  • qsub.MARCONI_SKL
  • qsub.MINT
  • qsub.NURION
  • qsub.OMEGA
PPPL-STAMPEDE
  • qsub.PPPL
  • qsub.PPPL_atom
  • qsub.PPPL_gcc
  • qsub.PSFCLUSTER
  • qsub.SATURN
  • qsub.SATURN_GCC
  • qsub.SHENMA
  • qsub.STAMPEDE
  • qsub.STAMPEDE2_KNL_HT2_IFORT
  • qsub.STAMPEDE2_KNL_IFORT
  • qsub.STAMPEDE2_SKX_IFORT
SUMMIT-MARCONI
  • qsub.SUMMIT
  • qsub.SUMMITDEV_GPU
  • qsub.PERLMUTTER_GPU
  • qsub.PERLMUTTER_GPU_80G
  • qsub.PERLMUTTER_CPU
  • qsub.PERLMUTTER_CPU_Cray
  • qsub.SHELL
  • qsub.MARCONI_100G
  • qsub.MARCONI_A

gacode/shared/bin

gacode_getversion-update_gacode.csh
  • gacode_getversion
  • gacode_mpi_tool
  • gacode_platforms
  • gacode_printversion
  • gacode_qsub
  • gacode_qsub_multi
  • gacode_reg
  • gacode_reg_do
  • gacode_regression.py
  • gacode_release_new_version.sh
  • gacode_setup
  • gacode_setup.tcsh
  • gacode_sim_warn
  • osxlist.py
  • summit_tool
  • update_gacode.csh

@wdeshazer wdeshazer changed the title Introduced user configurable memory specification Introduced user configurable memory specification to qsub Feb 6, 2024
elif [[ $USERMPN == false ]]; then
MEMPERNODE=
fi
export MEMP
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this is incomplete. Maybe

export MEMPERNODE
export MEMPERCPU

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smithsp I'm reopening the PR and will correct. I am actually refactoring so that defaults will be set in platform qsub configurations.

@wdeshazer
Copy link
Contributor Author

@jmcclena wants to ensure that we are not disrupting the running behavior for other institutions.

@jcandy jcandy merged commit 5c697f4 into master Feb 6, 2024
@wdeshazer
Copy link
Contributor Author

@jcandy I should have marked this as WIP. I apologize. I already found a typo and I need to run the regression tests. And also, I did want to get a sense from you on whether I should enable this for everyone. I can start a new branch to fix this one or if we can revert and I will keep working this one. @smithsp suggestions?

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