Skip to content

Updated template to not request GPU resources on CPU builds #1055

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Updated template to not request GPU resources on CPU builds #1055

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bd058f8
Updated template to not request GPU resources on CPU builds
prathi-wind Nov 19, 2025
985a861
Remove test code
prathi-wind Nov 19, 2025
3503f95
Fixed some issues and added abstraction variable
prathi-wind Nov 19, 2025
90e5f36
Removed unneeded include
prathi-wind Nov 19, 2025
214ee17
Fix indent on template
prathi-wind Nov 19, 2025
e5315b2
Fix documentation comment in m_bubbles module
sbryngelson Nov 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/simulation/m_bubbles.fpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

#:include 'macros.fpp'

!> @brief This module contains the procedures shared by the ensemble-averaged and volume-averaged bubble models.
!> @brief This module contains the procedures shared by ensemble-averaged and volume-averaged bubble models.
module m_bubbles

use m_derived_types !< Definitions of the derived types
Expand Down
4 changes: 2 additions & 2 deletions toolchain/templates/bridges2.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
% if account:
#SBATCH --account="${account}"
% endif
% if gpu:
% if is_gpu_active:
Copy link

@codeant-ai codeant-ai bot Nov 19, 2025

Choose a reason for hiding this comment

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

Suggestion: Use a guarded lookup such as globals().get('is_gpu_active', False) for the GPU block so that rendering still works even if only the old gpu flag is provided. [possible bug]

#SBATCH --gpu-bind=verbose,closest
#SBATCH --gres=gpu:v100-16:${tasks_per_node}
% endif
Expand All @@ -31,7 +31,7 @@ ${helpers.template_prologue()}

ok ":) Loading modules:\n"
cd "${MFC_ROOT_DIR}"
. ./mfc.sh load -c b -m ${'g' if gpu else 'c'}
. ./mfc.sh load -c b -m ${'g' if is_gpu_active else 'c'}
cd - > /dev/null
echo

Expand Down
2 changes: 1 addition & 1 deletion toolchain/templates/carpenter-cray.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ ${helpers.template_prologue()}

ok ":) Loading modules:\n"
cd "${MFC_ROOT_DIR}"
. ./mfc.sh load -c cc -m ${'g' if gpu else 'c'}
. ./mfc.sh load -c cc -m ${'g' if is_gpu_active else 'c'}
cd - > /dev/null
echo

Expand Down
2 changes: 1 addition & 1 deletion toolchain/templates/carpenter.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ ${helpers.template_prologue()}

ok ":) Loading modules:\n"
cd "${MFC_ROOT_DIR}"
. ./mfc.sh load -c c -m ${'g' if gpu else 'c'}
. ./mfc.sh load -c c -m ${'g' if is_gpu_active else 'c'}
cd - > /dev/null
echo

Expand Down
3 changes: 2 additions & 1 deletion toolchain/templates/default.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import os
from mako.exceptions import RuntimeException
%>

% if os.name == 'nt':
@echo off
% else:
Expand Down Expand Up @@ -48,7 +49,7 @@ if engine == 'batch':
(set -x; ${profiler} \
jsrun --nrs ${tasks_per_node*nodes} \
--cpu_per_rs 1 \
--gpu_per_rs ${1 if gpu else 0} \
--gpu_per_rs ${1 if is_gpu_active else 0} \
--tasks_per_rs 1 \
"${target.get_install_binpath(case)}")
elif [ "$binary" == "srun" ]; then
Expand Down
4 changes: 2 additions & 2 deletions toolchain/templates/delta.mako
View file
Open in desktop
Copy link
Contributor

@qodo-merge-pro qodo-merge-pro bot Nov 19, 2025

Choose a reason for hiding this comment

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

High-level Suggestion

The logic for checking GPU usage is duplicated across 14 template files. This should be centralized into a single helper function or variable to improve maintainability and consistency. [High-level, importance: 8]

Solution Walkthrough:

Before:

// In toolchain/templates/bridges2.mako
<%! from mfc.state import gpuConfigOptions %>
...
% if gpu != gpuConfigOptions.NONE.value:
  #SBATCH --gres=gpu:v100-16:${tasks_per_node}
% endif

// In toolchain/templates/delta.mako
<%! from mfc.state import gpuConfigOptions %>
...
% if gpu != gpuConfigOptions.NONE.value:
  #SBATCH --gpus-per-node=${tasks_per_node}
% endif

// ... and so on for many other template files

After:

// In a central Python file that renders templates:
from mfc.state import gpuConfigOptions
...
is_gpu_active = (gpu_mode != gpuConfigOptions.NONE.value)
template.render(..., is_gpu_active=is_gpu_active)


// In toolchain/templates/bridges2.mako (and all others)
// No import or complex check needed
...
% if is_gpu_active:
  #SBATCH --gres=gpu:v100-16:${tasks_per_node}
% endif

// In toolchain/templates/delta.mako (and all others)
...
% if is_gpu_active:
  #SBATCH --gpus-per-node=${tasks_per_node}
% endif

Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
% if account:
#SBATCH --account="${account}"
% endif
% if gpu:
% if gpu != gpuConfigOptions.NONE.value:
#SBATCH --gpus-per-node=${tasks_per_node}
#SBATCH --mem=208G
#SBATCH --gpu-bind=closest
Comment on lines +17 to 20
Copy link
Contributor

@qodo-merge-pro qodo-merge-pro bot Nov 19, 2025

Choose a reason for hiding this comment

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

Suggestion: To ensure consistency and prevent potential NameError exceptions, replace the direct GPU check in delta.mako with the centralized is_gpu_active variable. [possible issue, importance: 9]

New proposed code: 
-% if gpu != gpuConfigOptions.NONE.value:
+% if is_gpu_active:
 #SBATCH --gpus-per-node=${tasks_per_node}
 #SBATCH --mem=208G
 #SBATCH --gpu-bind=closest
 % endif

Expand All @@ -32,7 +32,7 @@ ${helpers.template_prologue()}

ok ":) Loading modules:\n"
cd "${MFC_ROOT_DIR}"
. ./mfc.sh load -c d -m ${'g' if gpu else 'c'}
. ./mfc.sh load -c d -m ${'g' if gpu != gpuConfigOptions.NONE.value else 'c'}
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

gpuConfigOptions is referenced here but the template no longer imports it, so rendering the delta SLURM template now fails with a NameError instead of launching jobs. Reintroduce the state import before using the enum value.

Prompt for AI agents 
Address the following comment on toolchain/templates/delta.mako at line 35:

<comment>`gpuConfigOptions` is referenced here but the template no longer imports it, so rendering the delta SLURM template now fails with a NameError instead of launching jobs. Reintroduce the state import before using the enum value.</comment>

<file context>
@@ -33,7 +32,7 @@ ${helpers.template_prologue()}
 ok &quot;:) Loading modules:\n&quot;
 cd &quot;${MFC_ROOT_DIR}&quot;
-. ./mfc.sh load -c d -m ${&#39;g&#39; if gpu else &#39;c&#39;}
+. ./mfc.sh load -c d -m ${&#39;g&#39; if gpu != gpuConfigOptions.NONE.value else &#39;c&#39;}
 cd - &gt; /dev/null
 echo
</file context>

✅ Addressed in 3503f95

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

gpuConfigOptions is used in the updated module-load command even though this template no longer imports it, so Delta submissions will crash with a NameError before running; reintroduce the import (or qualify the reference) so the CPU/GPU selection works.

Prompt for AI agents 
Address the following comment on toolchain/templates/delta.mako at line 35:

<comment>`gpuConfigOptions` is used in the updated module-load command even though this template no longer imports it, so Delta submissions will crash with a NameError before running; reintroduce the import (or qualify the reference) so the CPU/GPU selection works.</comment>

<file context>
@@ -33,7 +32,7 @@ ${helpers.template_prologue()}
 ok &quot;:) Loading modules:\n&quot;
 cd &quot;${MFC_ROOT_DIR}&quot;
-. ./mfc.sh load -c d -m ${&#39;g&#39; if gpu else &#39;c&#39;}
+. ./mfc.sh load -c d -m ${&#39;g&#39; if gpu != gpuConfigOptions.NONE.value else &#39;c&#39;}
 cd - &gt; /dev/null
 echo
</file context>
Fix with Cubic

Copy link
Contributor

@qodo-merge-pro qodo-merge-pro bot Nov 19, 2025

Choose a reason for hiding this comment

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

Suggestion: To maintain consistency and prevent potential errors, replace the inline GPU check with the is_gpu_active variable for module loading. [possible issue, importance: 9]

Suggested change
. ./mfc.sh load -c d -m ${'g' if gpu != gpuConfigOptions.NONE.value else 'c'}
. ./mfc.sh load -c d -m ${'g' if is_gpu_active else 'c'}

cd - > /dev/null
echo

Expand Down
4 changes: 2 additions & 2 deletions toolchain/templates/deltaai.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
% if account:
#SBATCH --account="${account}"
% endif
% if gpu:
% if is_gpu_active:
#SBATCH --gpus-per-node=${tasks_per_node}
#SBATCH --mem=208G
#SBATCH --gpu-bind=closest
Expand All @@ -32,7 +32,7 @@ ${helpers.template_prologue()}

ok ":) Loading modules:\n"
cd "${MFC_ROOT_DIR}"
. ./mfc.sh load -c dai -m ${'g' if gpu else 'c'}
. ./mfc.sh load -c dai -m ${'g' if is_gpu_active else 'c'}
cd - > /dev/null
echo

Expand Down
8 changes: 4 additions & 4 deletions toolchain/templates/frontier.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#SBATCH --time=${walltime}
#SBATCH --cpus-per-task=7
#SBATCH -C nvme
% if gpu:
% if is_gpu_active:
#SBATCH --gpus-per-task=1
#SBATCH --gpu-bind=closest
% endif
Expand All @@ -34,12 +34,12 @@ ${helpers.template_prologue()}
ok ":) Loading modules:\n"
cd "${MFC_ROOT_DIR}"
% if engine == 'batch':
. ./mfc.sh load -c f -m ${'g' if gpu else 'c'}
. ./mfc.sh load -c f -m ${'g' if is_gpu_active else 'c'}
% endif
cd - > /dev/null
echo

% if gpu:
% if is_gpu_active:
export MPICH_GPU_SUPPORT_ENABLED=1
% else:
export MPICH_GPU_SUPPORT_ENABLED=0
Expand All @@ -66,7 +66,7 @@ ulimit -s unlimited
% if engine == 'interactive':
--unbuffered --nodes ${nodes} --ntasks-per-node ${tasks_per_node} \
--cpus-per-task 7 \
% if gpu:
% if is_gpu_active:
--gpus-per-task 1 --gpu-bind closest \
% endif
${profiler} "${target.get_install_binpath(case)}")
Expand Down
4 changes: 2 additions & 2 deletions toolchain/templates/hipergator.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#SBATCH --output="${name}.out"
#SBATCH --time=${walltime}
#SBATCH --cpus-per-task=7
% if gpu:
% if is_gpu_active:
#SBATCH --gpus-per-task=1
#SBATCH --gpu-bind=closest
% endif
Expand All @@ -35,7 +35,7 @@ ${helpers.template_prologue()}
ok ":) Loading modules:\n"
cd "${MFC_ROOT_DIR}"
% if engine == 'batch':
. ./mfc.sh load -c h -m ${'g' if gpu else 'c'}
. ./mfc.sh load -c h -m ${'g' if is_gpu_active else 'c'}
% endif
cd - > /dev/null
echo
Expand Down
7 changes: 6 additions & 1 deletion toolchain/templates/include/helpers.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
<%! import os %>
<%!
import os
from mfc.state import gpuConfigOptions
%>

<% is_gpu_active = (gpu != gpuConfigOptions.NONE.value) %>

<%def name="template_prologue()">
% if os.name != 'nt':
Expand Down
4 changes: 2 additions & 2 deletions toolchain/templates/nautilus.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
% if account:
#SBATCH --account="${account}"
% endif
% if gpu:
% if is_gpu_active:
#SBATCH --gpu-bind=verbose,closest
#SBATCH --gres=gpu:v100-16:${tasks_per_node}
% endif
Expand All @@ -31,7 +31,7 @@ ${helpers.template_prologue()}

ok ":) Loading modules:\n"
cd "${MFC_ROOT_DIR}"
. ./mfc.sh load -c n -m ${'g' if gpu else 'c'}
. ./mfc.sh load -c n -m ${'g' if is_gpu_active else 'c'}
cd - > /dev/null
echo

Expand Down
4 changes: 2 additions & 2 deletions toolchain/templates/oscar.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
% if account:
#SBATCH --account="${account}"
% endif
% if gpu:
% if is_gpu_active:
Copy link

@codeant-ai codeant-ai bot Nov 19, 2025

Choose a reason for hiding this comment

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

Suggestion: Guard the SBATCH GPU directives with a context lookup so missing is_gpu_active values don't crash the template. [possible bug]

#SBATCH --gpus-per-node=${tasks_per_node}
#SBATCH --mem=64G
#SBATCH --gpu-bind=closest
Expand All @@ -32,7 +32,7 @@ ${helpers.template_prologue()}

ok ":) Loading modules:\n"
cd "${MFC_ROOTDIR}"
Copy link
Contributor

@qodo-merge-pro qodo-merge-pro bot Nov 19, 2025

Choose a reason for hiding this comment

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

Suggestion: Correct the typo in the environment variable from MFC_ROOTDIR to MFC_ROOT_DIR to prevent a script failure. [possible issue, importance: 10]

Suggested change
cd "${MFC_ROOTDIR}"
cd "${MFC_ROOT_DIR}"

. ./mfc.sh load -c o -m ${'g' if gpu else 'c'}
. ./mfc.sh load -c o -m ${'g' if is_gpu_active else 'c'}
cd - > /dev/null
echo

Expand Down
4 changes: 2 additions & 2 deletions toolchain/templates/phoenix-bench.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
% if quality_of_service:
#SBATCH --qos=${quality_of_service}
% endif
% if gpu:
% if is_gpu_active:
#SBATCH --gres=gpu:V100:${tasks_per_node}
#SBATCH --mem-per-gpu=16G\
% endif
Expand All @@ -31,7 +31,7 @@ ${helpers.template_prologue()}

ok ":) Loading modules:\n"
cd "${MFC_ROOT_DIR}"
. ./mfc.sh load -c p -m ${'g' if gpu else 'c'}
. ./mfc.sh load -c p -m ${'g' if is_gpu_active else 'c'}
cd - > /dev/null
echo

Expand Down
4 changes: 2 additions & 2 deletions toolchain/templates/phoenix.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
% if quality_of_service:
#SBATCH --qos=${quality_of_service}
% endif
% if gpu:
% if is_gpu_active:
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

is_gpu_active is referenced here, but the phoenix template never defines or imports that name (it only exists inside the helpers namespace), so rendering this template will raise NameError before the batch script is produced.

Prompt for AI agents 
Address the following comment on toolchain/templates/phoenix.mako at line 20:

<comment>`is_gpu_active` is referenced here, but the phoenix template never defines or imports that name (it only exists inside the helpers namespace), so rendering this template will raise `NameError` before the batch script is produced.</comment>

<file context>
@@ -18,7 +17,7 @@
 #SBATCH --qos=${quality_of_service}
 % endif
-% if gpu != gpuConfigOptions.NONE.value:
+% if is_gpu_active:
 #SBATCH --gres=gpu:V100:${tasks_per_node}
 #SBATCH --mem-per-gpu=16G\
</file context>
Fix with Cubic

#SBATCH --gres=gpu:V100:${tasks_per_node}
#SBATCH --mem-per-gpu=16G\
% endif
Comment on lines +20 to 23
Copy link

@coderabbitai coderabbitai bot Nov 19, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

GPU SBATCH directives updated, but trailing backslash needs removal.

The condition change from if gpu to if is_gpu_active is correct. However, line 22 has a trailing backslash (16G\) with no line continuation, which could cause issues with SLURM directive parsing.

Apply this diff to remove the trailing backslash:

-#SBATCH --mem-per-gpu=16G\
+#SBATCH --mem-per-gpu=16G
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
% if is_gpu_active:
#SBATCH --gres=gpu:V100:${tasks_per_node}
#SBATCH --mem-per-gpu=16G\
% endif
% if is_gpu_active:
#SBATCH --gres=gpu:V100:${tasks_per_node}
#SBATCH --mem-per-gpu=16G
% endif
🤖 Prompt for AI Agents 
In toolchain/templates/phoenix.mako around lines 20 to 23, the GPU SBATCH block
contains a trailing backslash on the --mem-per-gpu directive ("16G\") which
should be removed; edit that line to remove the backslash so it reads a normal
SLURM directive (e.g., "--mem-per-gpu=16G") and ensure no unintended line
continuation characters remain in the SBATCH directives.

Expand All @@ -31,7 +31,7 @@ ${helpers.template_prologue()}

ok ":) Loading modules:\n"
cd "${MFC_ROOT_DIR}"
. ./mfc.sh load -c p -m ${'g' if gpu else 'c'}
. ./mfc.sh load -c p -m ${'g' if is_gpu_active else 'c'}
cd - > /dev/null
echo

Expand Down
4 changes: 2 additions & 2 deletions toolchain/templates/santis.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ ${helpers.template_prologue()}
ok ":) Loading modules:\n"
cd "${MFC_ROOT_DIR}"
% if engine == 'batch':
. ./mfc.sh load -c san -m ${'g' if gpu else 'c'}
. ./mfc.sh load -c san -m ${'g' if is_gpu_active else 'c'}
% endif
cd - > /dev/null
echo
Expand All @@ -74,7 +74,7 @@ echo
--ntasks=${nodes*tasks_per_node} \
--cpus-per-task 72 \
--cpu-bind=none \
% if gpu:
% if is_gpu_active:
--gpus-per-task 1 \
% endif
--wait 200 --bcast=/tmp/${target.name} \
Expand Down
3 changes: 2 additions & 1 deletion toolchain/templates/summit.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

<%namespace name="helpers" file="helpers.mako"/>


% if engine == 'batch':
#BSUB -J {{{name}}}
#BSUB -nnodes {{{nodes}}}
Expand All @@ -16,7 +17,7 @@ ${helpers.template_prologue()}

ok ":) Loading modules:\n"
cd "${MFC_ROOT_DIR}"
. ./mfc.sh load -c s -m ${'g' if gpu else 'c'}
. ./mfc.sh load -c s -m ${'g' if is_gpu_active else 'c'}
cd - > /dev/null
echo

Expand Down
Loading