Add PBS scheduler plugin - #1093
Merged
Merged
Conversation
* fixed issue with -q not defaulting to normal (site-specific setting, but unavoidable, unfortunately)
* exclusive mode now works * custom filter ignores nodes in the queue if queue is set (was preventing series from starting properly) * qsub should be added to kickoff line to verify * select= number accurately represents nodes that match the criteria (free, job-exclusive and in queue). was stopping jobs from queuing (what we wanted) behind other jobs to run in the queue
Adds a scheduler plugin for PBS/OpenPBS, submitting jobs via qsub. This addresses the review feedback on #928 ('should add some unit tests and update docs') and rebases the work onto current develop. Plugin (lib/pavilion/schedulers/plugins/pbs.py): - Advanced scheduler plugin; node inventory from 'pbsnodes', job status from 'qstat', cancellation via 'qdel'. - Select statement is built as '-l select=<nodes>:ncpus=<tasks>[:host=][:mpiprocs=][:model=]'. - 'mpiprocs' is optional and unset by default: some sites require it in the select statement and others reject it, so when unset it is omitted entirely and the submitted select statement is unchanged. Note 'tasks' maps to ncpus (cores per chunk), which is distinct from mpiprocs (ranks per chunk) -- mpiprocs is what determines the PBS_NODEFILE line count and the rank count mpirun sees. - Scheduler variables: queue, walltime, mpiprocs, test_cmd. Each returns '' rather than None when unset, as dfr_var_method rejects a None return. - srun_args is overridden to '' -- it is inherited from SchedulerVariables and composes Slurm flags, so under PBS it would otherwise silently hand a test Slurm arguments rather than erroring. - EXAMPLE is populated for every deferred variable, as required by sched_tests.SchedTests.test_check_examples. Tests (test/tests/pbs_sched_tests.py): 18 tests covering scheduler variables, node list parsing and deduplication, raw node data transformation across free/busy/down states, qsub header generation, value converters, state validation and config defaults. All avoid invoking 'pbsnodes'/'qstat', so they run without a PBS installation -- the variable class is instantiated directly, the same approach sched_tests uses to cover every scheduler. Docs: a PBS section in docs/tests/scheduling.rst covering the scheduler specific options and variables, plus the scheduler listing and a CHANGELOG entry. Verified on Python 3.6.8 (a supported version) against an untouched develop baseline: 433 -> 451 passing, with the same pre-existing failures. See the note below regarding show_cmd_tests. Provenance: Claude Code (Claude Opus 5, Anthropic) Workflow: collaborative
Brings the PBS scheduler plugin branch up to date with develop (41 commits) and addresses the review feedback on #928: 'should add some unit tests and update docs'. Adds: - test/tests/pbs_sched_tests.py -- 18 unit tests covering scheduler variables, PBS_NODEFILE parsing and deduplication, raw node data transformation across free/busy/down states, qsub header generation, value converters, state validation and config defaults. None of them invoke pbsnodes/qstat/qsub, so they run in CI without a PBS installation. - A PBS section in docs/tests/scheduling.rst documenting the scheduler specific options and variables, plus the scheduler listing entry. - An optional 'mpiprocs' select key, unset by default so sites that do not use it submit an unchanged select statement. - EXAMPLE entries for every deferred scheduler variable, required by sched_tests.SchedTests.test_check_examples. - An srun_args override returning '', since it is inherited from SchedulerVariables and composes Slurm flags that are meaningless under PBS. The release note moved from RELEASE.txt to CHANGELOG.md, which develop renamed since this branch was cut. Verified on Python 3.6.8 against an untouched develop baseline: 433 -> 451 passing with the same pre-existing failures. Note: test_show_format_list_parsable in show_cmd_tests asserts a hardcoded scheduler count of 4, which becomes 5 once PBS is registered. That test is left unchanged here and will need updating. Provenance: Claude Code (Claude Opus 5, Anthropic) Workflow: collaborative
CI's 'Run style checks' job has been failing on this branch since April, and
because the unit test jobs are gated behind it they were never running at all --
the failure emails say 'unit tests' but the tests report 'Skipped: 473 -- 100%'.
pylint 2.13.9 reported 13 messages, all in pbs.py:
invalid-name (6) s, nf, n, n, n, t
no-self-use (6) methods that do not reference self
bad-indentation (1) line 468 used 15 spaces where its siblings use 16
Fixes:
- Renamed the short locals: s -> state, nf -> nodefile_handle,
n_tmp -> raw_nodes, n -> node / node_count, t -> tasks.
- Corrected the indentation in the all_queue_nodes branch of _kickoff and
wrapped the two now-longer select format calls.
- _pbsnodes_parse and _qstat are genuine local helpers that never use self, so
they are now @staticmethod. Calls through self still resolve normally.
- _get_raw_node_data, _filter_custom, _available and cancel are required
overrides of the scheduler plugin base classes, so they must remain instance
methods; no-self-use is a false positive there and is disabled per method
with a comment explaining why.
No behavioural change: the 18 PBS unit tests and the 30 scheduler tests all still
pass. Verified with pylint that invalid-name and bad-indentation are now clean.
Note: the 'disable=no-self-use' comments are correct for the pinned pylint
2.13.9. Should pylint ever be bumped past 2.14, no-self-use moves to an optional
extension and those comments would themselves raise useless-option-value.
Provenance: Claude Code (Claude Opus 5, Anthropic)
Workflow: collaborative
The test asserted a hardcoded count of 4 lines from 'pav show schedulers --format list'. Registering an additional scheduler plugin makes it 5, so the test fails for any change that adds a scheduler -- it failed here once the PBS plugin was registered. The point of the test is that the output is parsable and carries no header, so the literal count is incidental. It now derives the expectation from len(schedulers.list_plugins()), which keeps the real assertions intact (one line per scheduler, no header) without breaking whenever the scheduler set changes. Verified both ways: passes with the PBS plugin registered (5 schedulers) and on an unmodified develop checkout with 4. Provenance: Claude Code (Claude Opus 5, Anthropic) Workflow: collaborative
hwikle-lanl
approved these changes
Aug 25, 2026
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
4 participants
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.
Adds a scheduler plugin for PBS/OpenPBS.
Supersedes #928, which I wasn't able to update. Background discussion in #922.
Jobs are submitted with
qsub; node inventory comes frompbsnodes, status fromqstat, and cancellation fromqdel. The select statement is built as:Notes for review
mpiprocsis optional and unset by default. Some sites require it in theselect statement and others reject it, so when it isn't set it's omitted
entirely and the submitted select statement is unchanged. Worth noting
tasksmaps to
ncpus(cores per chunk), which is a different resource frommpiprocs(ranks per chunk) — the latter is what determines the number oflines in
$PBS_NODEFILEand therefore the rank countmpirunsees.Scheduler variables return
''rather thanNonewhen unset. Pavilion'sdfr_var_methodrejects aNonereturn and crashes kickoff, so the emptystring is the correct "not set" sentinel.
srun_argsis overridden to''. It's inherited fromSchedulerVariablesand composes Slurm flags (--account,--partition,--qos, ...). Without the override, a PBS test referencing{{sched.srun_args}}would silently be handed Slurm arguments instead ofgetting an error.
This PR modifies one unrelated test.
test_show_format_list_parsableinshow_cmd_tests.pyasserted a hardcoded count of 4 schedulers, so registeringany new scheduler breaks it. It now derives the expected count from
len(schedulers.list_plugins()), which keeps the real assertions intact (oneline per scheduler, no header) without breaking again the next time a scheduler
is added. Verified passing both with this plugin registered (5) and on an
unmodified
developcheckout (4). Happy to split that into its own PR if you'dprefer it separate.
Testing
test/tests/pbs_sched_tests.pyadds 18 unit tests covering schedulervariables,
PBS_NODEFILEparsing and deduplication, node data transformationacross free/busy/down states, qsub header generation, value converters, state
validation and config defaults.
None of them invoke
pbsnodes/qstat/qsub, so they run on a CI runner withno PBS installed. That means CI covers the plugin's logic; job submission
itself was verified separately against a real PBS cluster, including select
statement contents checked against the PBS accounting log.
CI is green on Python 3.6, 3.10 and 3.12.
Code review checklist: