Let an app version declare its own sbatch parameters - #584
Draft
leoschwarz wants to merge 4 commits into
Draft
Conversation
An app version may now carry extra sbatch flags in its app.yml, which the slurm submitter merges between its own defaults and the per-workunit parameters. A null value removes a flag the submitter would otherwise pass. Reading the app spec at submit time never fails a submission: any problem falls back to the submitter defaults with a warning, since the job itself reports app spec errors with more context. Flags that carry the job's logging, working directory and environment are reserved, as are workunit variables, which are not in scope when an app spec is evaluated.
Adds a task-shaped section to the app guide (what to write, what a user can still override, how to unset a flag, which flags are refused, how to check it took effect) plus the field-level reference and a changelog entry.
Two silent-wrongness cases, both caught at spec validation: - YAML parses an unquoted 24:00:00 as the integer 86400, and sbatch reads a bare integer --time as minutes, so an app asking for a day would silently get 60. - Each flag renders as one #SBATCH line, so a newline in a value would append lines to the generated job script.
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
1 participant
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.
Every app currently gets whatever resources its submitter deployment was configured with, so there is no way to say "this app needs 24 cores and that one needs 1" short of running a submitter per app. An app version can now carry
submitter_params— plain sbatch flags — which the SLURM submitter merges into the job it generates.This restores the
paramshalf of theSubmitterReffield deleted in c778d65 ("Modernize submitter integration", #207), this time actually read by the submitter. Thenamehalf and theSubmittersSpecregistry are deliberately not restored: B-Fabric decides which submitter runs a workunit, so a name could only select a profile inside the submitter already running, and the app version is a better selector for the same job — adevelversion can carry different params than the released one, with no registry to keep in sync per environment.Key points:
nullvalue removes a flag the submitter would otherwise pass, which is the only way to unset one.application_versionall fall back to the submitter defaults with a warning. A submission must not fail over the app spec, since the job itself reports spec problems with far more context.validate app-speccatches them before deployment:--output/--error/--chdir/--export(the submitter owns the job's logging, working directory and environment),${workunit...}values (no workunit is in scope when a spec is evaluated), integer--time(YAML reads an unquoted24:00:00as86400, which sbatch reads as minutes), and multi-line values (each flag becomes one#SBATCHline).Trade-off worth a reviewer's opinion:
RESERVED_SBATCH_FLAGSlives in the spec module, sospecs/now encodes something the submitter owns. The alternative was dropping those flags at merge time with a warning; validation-time rejection was chosen because a warning in a submitter log is not seen by the person editing the app.yml.Out of scope here: adding
cpus_per_taskto the workunit whitelist, and translating a multi-nodenodelistinto--excludeof the partition complement.Verification: 79 tests in
tests/bfabric_app_runner/specsand.../bfabric_integration, basedpyright clean. The 6 failures indispatch/test_dispatch_resource_flow.pyare pre-existing on main (pandera on Python 3.14).🤖 Prepared with assistance from Claude Opus 5 via Claude Code.