fix(submit): honor #SBATCH/#PJM directives in user-supplied scripts#11
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes hpc submit -s/--script so scheduler directives (#SBATCH / #PJM) written in the user-provided script are not accidentally placed after executable lines in the rendered wrapper script (where Slurm/PJM stop scanning directives).
Changes:
- Add
_extract_prologue_directives()to hoist column-zero scheduler directives from the user script’s prologue into the rendered job-script prologue. - Update job-script rendering for both tracked runs (
submit_run/_render_job_script) and the legacysubmit_jobpath to include hoisted user directives. - Add unit + integration tests covering directive extraction and ordering/precedence behavior; document the behavior in README and the Claude skill.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/hpc/job.py |
Implements directive hoisting and renders user directives in the job prologue for both submission paths. |
tests/test_job.py |
Adds unit tests for the extractor and integration tests asserting correct prologue ordering/precedence. |
README.md |
Documents that script-top directives are honored and explains precedence semantics. |
.claude/skills/hpc/SKILL.md |
Updates the “writing scripts” guidance to mention directive hoisting and precedence. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The job-script template placed user content below `cd workdir` and env-setup lines, where `sbatch` and `pjsub` have already stopped scanning prologue directives. Directives written at the top of a script passed via `hpc submit -s script.sh` were therefore silently dropped. Hoist column-zero `#SBATCH` (Slurm) / `#PJM` (PJM) lines from the user script's prologue into the rendered job-script prologue, mirroring the schedulers' own prologue-scan rule (shebang + blank + comment + matching directive lines, stopping at the first executable line). Directive-look-alike content past the first executable line or inside heredocs is left in the body untouched. User-supplied directives are emitted after config-derived ones so they win on conflict via last-occurrence-wins; hpc's bookkeeping `--output` / `--error` lines stay last so run-tracking output paths are not overrideable.
PJM `options` is parsed as a list of lists, so it cannot live under a TOML table header `[pjm.options]` — that syntax is for tables. Refer to it as the `pjm.options` array (dotted-path) instead.
The previous extractor treated indented `# comment` lines and whitespace-only lines as bash-style comments/blanks and continued the prologue scan past them. Slurm and PJM are stricter: only column-zero `#` lines are comments, and only truly-blank lines (`\n` / `\r\n`) are blank. With the looser rule, hpc could hoist a directive that the scheduler would have ignored if the script were submitted directly via `sbatch script.sh` / `pjsub script.sh`. Treat indented comments and whitespace-only lines as scan- terminating, matching the schedulers' own column-sensitive rule.
ultimatile
force-pushed
the
fix/6-script-directives
branch
from
546e902 to
82d57b5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #6.
Summary
#SBATCH(Slurm) /#PJM(PJM) directives written in scripts passed viahpc submit -s script.shwere silently ignored. The job-script template placed user content belowcd workdirand env-setup lines, past the point wheresbatch/pjsubstop scanning prologue directives — so user directives ended up as ordinary comments.Root cause
src/hpc/job.py'sJOB_TEMPLATErendered the user-supplied script content ({{ cmd }}) at the bottom, after the executablecd job_workdirline. Slurm and PJM both stop scanning#SBATCH/#PJMdirectives at the first non-comment, non-blank, non-directive line of the script, so the user's directives were never seen.Fix
Hoist column-zero
^#SBATCH\b/^#PJM\blines out of the user script's prologue and emit them in the rendered job-script prologue. The extractor mirrors the schedulers' own prologue-scan rule:#!shebang is dropped (the template emits its own)User-supplied directives are emitted after config-derived
[slurm.options]/[pjm.options]so they win on conflict via the schedulers' last-occurrence-wins semantics. The template's hardcoded--output=/--error=lines stay last, so hpc's run-tracking output paths are not user-overrideable.Test plan
_extract_prologue_directiveshelper unit tests (13 cases): empty input, shebang only / shebang + body, no shebang, Slurm and PJM extraction, prefix isolation between schedulers, post-executable directive not hoisted, heredoc directive-look-alike not hoisted, blank lines between directives preserved, non-directive comments kept in body, indented directive not hoisted, missing trailing newline preserved_render_job_script(5 cases): Slurm hoist with bookkeeping ordering, PJM multi-directive hoist with order preservation, conflict ordering for both schedulers, no-directive no-opsubmit_jobpath also hoists user directives (consistency between the two submission code paths)pyright src/hpc/: 0 errors, 0 warnings;ruff check,ruff format --check: cleanOut of scope
--output=/--error=lines emitted by the template currently use Slurm syntax (PJM expects-o/-e). Pre-existing bug, separate issue.cd job_workdirwith Slurm--chdir/ PJM-dwould be an architectural cleanup but does not by itself fix the prologue-scan-termination problem (env-setup lines remain). Future work.