Skip to content

feat: warn when SLURM ranks double count a node - #1390

Open
davidberenstein1957 wants to merge 1 commit into
masterfrom
feat/slurm-multirank-warning
Open

feat: warn when SLURM ranks double count a node#1390
davidberenstein1957 wants to merge 1 commit into
masterfrom
feat/slurm-multirank-warning

Conversation

@davidberenstein1957

Copy link
Copy Markdown
Collaborator

Split out of #1366, which mixed it in with the job-metadata columns. It is unrelated scope and stands on its own.

The hazard

On SLURM, srun --ntasks-per-node=4 starts four ranks on one node. With tracking_mode="machine" — the default — each of those four trackers measures the whole node, so the job's total is silently 4x the truth. Nothing in the output says so; the numbers just look wrong in a way nobody notices.

What this does

codecarbon/core/slurm.py reads SLURM_NTASKS_PER_NODE at tracker init and, in machine mode with more than one rank per node, logs a warning naming the two ways out: elect one tracker per node (SLURM_LOCALID == 0) or switch to tracking_mode="process".

That is the whole change: ~20 lines, os.environ and a regex, no dependency, no config key, no new output field. 4(x2) (heterogeneous allocations) parses as 4.

Not in this PR

Automatic rank election. Suppressing the extra trackers is the real fix, but it is the same problem as LOCAL_RANK == 0 in distributed training and deserves one generic solution with a caller asking for it. Warning first.

Tests

tests/test_slurm.py: warns for 4 and for 4(x2); silent for 1, for no SLURM variable at all, and for tracking_mode="process".

uv run pytest tests/test_slurm.py tests/test_emissions_tracker.py -q
33 passed

Based on #1370, same base as #1366.

🤖 Generated with Claude Code

@davidberenstein1957
davidberenstein1957 requested a review from a team as a code owner August 16, 2026 08:31
@codecov

codecov Bot commented Aug 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.45%. Comparing base (3ec31a0) to head (dae06ff).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1390      +/-   ##
==========================================
+ Coverage   91.43%   91.45%   +0.02%     
==========================================
  Files          49       50       +1     
  Lines        5057     5069      +12     
==========================================
+ Hits         4624     4636      +12     
  Misses        433      433              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@davidberenstein1957
davidberenstein1957 force-pushed the feat/slurm-multirank-warning branch from 10815c4 to cfd15f4 Compare August 19, 2026 09:08
@davidberenstein1957
davidberenstein1957 force-pushed the feat/slurm-multirank-warning branch from cfd15f4 to 89b00c5 Compare August 19, 2026 13:18
@github-actions github-actions Bot added size/M and removed size/L labels Aug 19, 2026
@davidberenstein1957
davidberenstein1957 changed the base branch from fix/csv-update-dtype-coercion to master August 19, 2026 13:31
With several ranks per node and tracking_mode="machine", every rank measures
the whole node, so the job's reported footprint is silently multiplied by the
rank count. Read SLURM_NTASKS_PER_NODE at tracker init and say so, since a
docs warning does not catch anyone.

Split out of #1366, where it was unrelated scope.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@davidberenstein1957
davidberenstein1957 force-pushed the feat/slurm-multirank-warning branch from 89b00c5 to dae06ff Compare August 19, 2026 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant