Deterministic run layout and {run_dir} placeholder - #354
Closed
atnair-amd wants to merge 3 commits into
Closed
atnair-amd wants to merge 3 commits into
atnair-amd wants to merge 3 commits into
Conversation
Every rank of a Slurm/Spur job step must derive the same shared-filesystem
paths: worker ranks never enter pytest, they read rank0's port out of
agent_dir. RunLayout resolves workspace/run_dir/agent_dir once, before pytest
or agent bootstrap, from inputs every rank shares.
- cvs/core/run_layout.py: the layout, cached per process so a timestamped
run_id cannot drift between callers.
- cvs run --workspace, resolved after the test name validates so a mistyped
suite does not litter shared storage.
- {run_dir} in test configs, published as CVS_RUN_DIR. utils_lib reads the
environment rather than importing run_layout, which would be circular via
cvs/core/__init__.py -> orchestrator factory -> utils_lib.
run_id keys off the job-step environment rather than is_managed_compute():
that predicate also probes for the scontrol/spur binaries, which are absent
from the container image even when srun exported the SLURM_* variables, so
ranks would each fall back to their own wall clock and diverge. It includes
SLURM_STEP_ID and the restart count because the job id alone is not unique
per run - concurrent steps in one allocation share it, and a requeue repeats
it.
Test configs resolve {run_dir} to a host path, but tests execute inside
the container via docker exec, so without a passthrough the substituted
path resolves to nothing on the container side and artifacts land in a
disposable overlay.
Mount it as an identity mapping -- any other container path would leave
the already-substituted value pointing elsewhere. RunLayout gains a
non-raising accessor because suites are also launched with bare pytest,
which never resolves a layout; in that case no mount is added.
Outside a scheduler-managed job nothing guarantees the workspace is reachable from the node the container is launched on, and bind-mounting a path the node lacks quietly materializes a root-owned empty directory instead of failing. RunLayout now records whether a scheduler launched the run, which it already had to determine for the run id, and the orchestrator mounts only when it did.
Collaborator
Author
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.
Summary
Every rank of a Slurm/SPUR job step has to derive the same run directory without coordinating, because those paths are the rendezvous for the agent control plane: worker ranks never enter pytest and read rank0's port out of
agent_dir.RunLayoutresolves workspace / run_dir / agent_dir once incvs run, before pytest or agent bootstrap, and publishes it asCVS_RUN_DIR.Ticket: Fixes AIMVT-300 (epic AIMVT-297)
Change
--workspace, falling back to$CVS_WORKSPACEthen to acvs_runsdirectory beside the venv. Run directory is<workspace>/cvs/runs/<run_id>.<job>.<step>[.r<restart>]under a scheduler, elselocal-<timestamp>. The job id alone is not enough: concurrent steps in one allocation share it and would share anagent_dir, and a requeued job repeats it.SLURM_*environment rather thanis_managed_compute(), which also probes for thescontrol/spurbinaries. Those are absent from the CVS container image even when srun exported the variables, so ranks would each fall back to their own wall clock and hang at the rendezvous instead of erroring.{run_dir}placeholder for test configs. Resolved from the environment becausecvs/lib/utils_lib.pycannot importcvs.coreat module level — the orchestrator factory imports it back. Using it outsidecvs runexits with a diagnostic rather than substituting an empty path.agent_dir(AIMVT-297).Tests
50 unit tests across
cvs/core/unittests/test_run_layout.py,test_container.py,test_run_plugin.pyandtest_utils_lib.py, covering rank agreement, requeue and concurrent-step collisions, unwritable workspaces, and the no-venv case.Gate:
make fmt-check,make lint(pylint 10.00/10),make ut— 1245 tests, all passing.