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

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

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

Show file tree
Hide file tree
Changes from 3 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
5 changes: 3 additions & 2 deletions toolchain/templates/bridges2.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env bash

<%namespace name="helpers" file="helpers.mako"/>
<%! from mfc.state import gpuConfigOptions %>

% if engine == 'batch':
#SBATCH --nodes=${nodes}
Expand All @@ -14,7 +15,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 +32,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
4 changes: 3 additions & 1 deletion toolchain/templates/carpenter-cray.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#!/usr/bin/env bash

<%namespace name="helpers" file="helpers.mako"/>
<%! from mfc.state import gpuConfigOptions %>


Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

[nitpick] Inconsistent blank line spacing after the import statement. This file has two blank lines after <%! from mfc.state import gpuConfigOptions %> (lines 5-6), while other template files like phoenix.mako, frontier.mako, etc. have only one blank line. For consistency, remove one of the blank lines.

Suggested change

Copilot uses AI. Check for mistakes.
% if engine == 'batch':
#PBS -l select=${nodes}:ncpus=192:mpiprocs=${tasks_per_node}
Expand All @@ -25,7 +27,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
4 changes: 3 additions & 1 deletion toolchain/templates/carpenter.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#!/usr/bin/env bash

<%namespace name="helpers" file="helpers.mako"/>
<%! from mfc.state import gpuConfigOptions %>


Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

[nitpick] Inconsistent blank line spacing after the import statement. This file has two blank lines after <%! from mfc.state import gpuConfigOptions %> (lines 5-6), while other template files like phoenix.mako, frontier.mako, etc. have only one blank line. For consistency, remove one of the blank lines.

Suggested change

Copilot uses AI. Check for mistakes.
% if engine == 'batch':
#PBS -l select=${nodes}:ncpus=192:mpiprocs=${tasks_per_node}
Expand All @@ -25,7 +27,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
4 changes: 3 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,8 @@
import os
from mako.exceptions import RuntimeException
%>
<%! from mfc.state import gpuConfigOptions %>
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

[nitpick] The import statement is placed in a separate <%! ... %> block from the existing imports (lines 1-4). For consistency and cleaner code organization, consider consolidating all module-level imports into a single <%! ... %> block:

<%!
import os
from mako.exceptions import RuntimeException
from mfc.state import gpuConfigOptions
%>
Suggested change
%>
<%! from mfc.state import gpuConfigOptions %>
from mfc.state import gpuConfigOptions
%>

Copilot uses AI. Check for mistakes.

% if os.name == 'nt':
@echo off
% else:
Expand Down Expand Up @@ -48,7 +50,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
5 changes: 3 additions & 2 deletions toolchain/templates/delta.mako
View file
Open in desktop
Copy link
Contributor

@qodo-code-review qodo-code-review 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
@@ -1,6 +1,7 @@
#!/usr/bin/env bash

<%namespace name="helpers" file="helpers.mako"/>
<%! from mfc.state import gpuConfigOptions %>

% if engine == 'batch':
#SBATCH --nodes=${nodes}
Expand All @@ -14,7 +15,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 +33,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 is_gpu_active else 'c'}
cd - > /dev/null
echo

Expand Down
5 changes: 3 additions & 2 deletions toolchain/templates/deltaai.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env bash

<%namespace name="helpers" file="helpers.mako"/>
<%! from mfc.state import gpuConfigOptions %>

% if engine == 'batch':
#SBATCH --nodes=${nodes}
Expand All @@ -14,7 +15,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 +33,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
9 changes: 5 additions & 4 deletions toolchain/templates/frontier.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env bash

<%namespace name="helpers" file="helpers.mako"/>
<%! from mfc.state import gpuConfigOptions %>

% if engine == 'batch':
#SBATCH --nodes=${nodes}
Expand All @@ -10,7 +11,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 +35,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 +67,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
5 changes: 3 additions & 2 deletions toolchain/templates/hipergator.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env bash

<%namespace name="helpers" file="helpers.mako"/>
<%! from mfc.state import gpuConfigOptions %>

% if engine == 'batch':
#SBATCH --nodes=${nodes}
Expand All @@ -9,7 +10,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 +36,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
1 change: 1 addition & 0 deletions toolchain/templates/include/helpers.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<%! import os %>

<% is_gpu_active = (gpu != gpuConfigOptions.NONE.value) %>
<%def name="template_prologue()">
% if os.name != 'nt':
#>
Expand Down
5 changes: 3 additions & 2 deletions toolchain/templates/nautilus.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env bash

<%namespace name="helpers" file="helpers.mako"/>
<%! from mfc.state import gpuConfigOptions %>

% if engine == 'batch':
#SBATCH --nodes=${nodes}
Expand All @@ -14,7 +15,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 +32,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
5 changes: 3 additions & 2 deletions toolchain/templates/oscar.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env bash

<%namespace name="helpers" file="helpers.mako"/>
<%! from mfc.state import gpuConfigOptions %>

% if engine == 'batch':
#SBATCH --nodes=${nodes}
Expand All @@ -14,7 +15,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 +33,7 @@ ${helpers.template_prologue()}

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

@qodo-code-review qodo-code-review 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
5 changes: 3 additions & 2 deletions toolchain/templates/phoenix-bench.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env bash

<%namespace name="helpers" file="helpers.mako"/>
<%! from mfc.state import gpuConfigOptions %>

% if engine == 'batch':
#SBATCH --nodes=${nodes}
Expand All @@ -17,7 +18,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 +32,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
5 changes: 3 additions & 2 deletions toolchain/templates/phoenix.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env bash

<%namespace name="helpers" file="helpers.mako"/>
<%! from mfc.state import gpuConfigOptions %>

% if engine == 'batch':
#SBATCH --nodes=${nodes}
Expand All @@ -17,7 +18,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
Contributor

@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 +32,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
5 changes: 3 additions & 2 deletions toolchain/templates/santis.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env bash

<%namespace name="helpers" file="helpers.mako"/>
<%! from mfc.state import gpuConfigOptions %>

% if engine == 'batch':
#SBATCH --uenv=icon/25.2:v1@santis
Expand Down Expand Up @@ -59,7 +60,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 +75,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
@@ -1,6 +1,7 @@
#!/usr/bin/env bash

<%namespace name="helpers" file="helpers.mako"/>
<%! from mfc.state import gpuConfigOptions %>

% if engine == 'batch':
#BSUB -J {{{name}}}
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