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 2 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
3 changes: 2 additions & 1 deletion 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 gpu != gpuConfigOptions.NONE.value:
#SBATCH --gpu-bind=verbose,closest
#SBATCH --gres=gpu:v100-16:${tasks_per_node}
% endif
Expand Down
2 changes: 2 additions & 0 deletions 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 Down
2 changes: 2 additions & 0 deletions 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 Down
2 changes: 2 additions & 0 deletions 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
3 changes: 2 additions & 1 deletion 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 gpu != gpuConfigOptions.NONE.value:
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.

This condition still allocates GPUs for OpenMP-only builds; it should only request GPUs when the configuration is explicitly the accelerator option.

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

<comment>This condition still allocates GPUs for OpenMP-only builds; it should only request GPUs when the configuration is explicitly the accelerator option.</comment>

<file context>
@@ -14,7 +15,7 @@
 #SBATCH --account=&quot;${account}&quot;
 % endif
-% if gpu:
+% if gpu != gpuConfigOptions.NONE.value:
 #SBATCH --gpus-per-node=${tasks_per_node}
 #SBATCH --mem=208G
</file context>
Suggested change
% if gpu != gpuConfigOptions.NONE.value:
% if gpu == gpuConfigOptions.ACC.value:
Fix with Cubic

#SBATCH --gpus-per-node=${tasks_per_node}
#SBATCH --mem=208G
#SBATCH --gpu-bind=closest
Comment on lines +17 to 20
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: 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 Down
3 changes: 2 additions & 1 deletion 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 gpu != gpuConfigOptions.NONE.value:
#SBATCH --gpus-per-node=${tasks_per_node}
#SBATCH --mem=208G
#SBATCH --gpu-bind=closest
Expand Down
7 changes: 4 additions & 3 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 gpu != gpuConfigOptions.NONE.value:
#SBATCH --gpus-per-task=1
#SBATCH --gpu-bind=closest
% endif
Expand Down Expand Up @@ -39,7 +40,7 @@ cd "${MFC_ROOT_DIR}"
cd - > /dev/null
echo

% if gpu:
% if gpu != gpuConfigOptions.NONE.value:
Copy link

@cursor cursor bot Nov 19, 2025

Choose a reason for hiding this comment

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

Bug: GPU load command uses stale boolean check

The expression . ./mfc.sh load -c f -m ${'g' if gpu else 'c'} still uses the old boolean check for gpu. Since gpu is now a string ('no', 'acc', or 'mp'), the string 'no' evaluates to truthy, causing CPU builds to incorrectly load GPU modules. Should be updated to ${'g' if gpu != gpuConfigOptions.NONE.value else 'c'} to match other fixed conditions in this PR.

Fix in Cursor Fix in Web

export MPICH_GPU_SUPPORT_ENABLED=1
% else:
export MPICH_GPU_SUPPORT_ENABLED=0
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Nov 19, 2025

Choose a reason for hiding this comment

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

P1 Badge Update remaining GPU conditionals to handle enum values

Changing the job header to gpu != gpuConfigOptions.NONE.value avoids requesting GPUs via SLURM, but other logic in this template still relies on if gpu (e.g. the module load line . ./mfc.sh load -c f -m ${'g' if gpu else 'c'}). Because gpu is now a non‑empty string ('no', 'acc', etc.), those expressions are always truthy and CPU builds will still load GPU modules and pass GPU runtime flags. This leaves CPU jobs misconfigured despite the header change. Please apply the same explicit comparison for the module selection here and in the other templates touched by this commit (bridges2.mako, carpenter*.mako, default.mako, hipergator.mako, summit.mako, etc.).

Useful? React with 👍 / 👎.

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 gpu != gpuConfigOptions.NONE.value:
--gpus-per-task 1 --gpu-bind closest \
% endif
${profiler} "${target.get_install_binpath(case)}")
Expand Down
3 changes: 2 additions & 1 deletion 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 gpu != gpuConfigOptions.NONE.value:
#SBATCH --gpus-per-task=1
#SBATCH --gpu-bind=closest
% endif
Expand Down
3 changes: 2 additions & 1 deletion 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 gpu != gpuConfigOptions.NONE.value:
#SBATCH --gpu-bind=verbose,closest
#SBATCH --gres=gpu:v100-16:${tasks_per_node}
% endif
Expand Down
3 changes: 2 additions & 1 deletion 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 gpu != gpuConfigOptions.NONE.value:
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.

This condition still requests GPUs for OpenMP CPU builds (gpu == gpuConfigOptions.MP.value); it should only request GPUs for the ACC (actual GPU) mode to avoid consuming GPU resources on CPU jobs.

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

<comment>This condition still requests GPUs for OpenMP CPU builds (gpu == gpuConfigOptions.MP.value); it should only request GPUs for the ACC (actual GPU) mode to avoid consuming GPU resources on CPU jobs.</comment>

<file context>
@@ -14,7 +15,7 @@
 #SBATCH --account=&quot;${account}&quot;
 % endif
-% if gpu:
+% if gpu != gpuConfigOptions.NONE.value:
 #SBATCH --gpus-per-node=${tasks_per_node}
 #SBATCH --mem=64G
</file context>
Suggested change
% if gpu != gpuConfigOptions.NONE.value:
% if gpu == gpuConfigOptions.ACC.value:
Fix with Cubic

#SBATCH --gpus-per-node=${tasks_per_node}
#SBATCH --mem=64G
#SBATCH --gpu-bind=closest
Expand Down
3 changes: 2 additions & 1 deletion 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 gpu != gpuConfigOptions.NONE.value:
#SBATCH --gres=gpu:V100:${tasks_per_node}
#SBATCH --mem-per-gpu=16G\
% endif
Expand Down
3 changes: 2 additions & 1 deletion 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 gpu != gpuConfigOptions.NONE.value:
#SBATCH --gres=gpu:V100:${tasks_per_node}
#SBATCH --mem-per-gpu=16G\
% endif
Expand Down
3 changes: 2 additions & 1 deletion 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 @@ -74,7 +75,7 @@ echo
--ntasks=${nodes*tasks_per_node} \
--cpus-per-task 72 \
--cpu-bind=none \
% if gpu:
% if gpu != gpuConfigOptions.NONE.value:
--gpus-per-task 1 \
% endif
--wait 200 --bcast=/tmp/${target.name} \
Expand Down
1 change: 1 addition & 0 deletions 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 Down