[gchlebus]

Problem

TrainDataProcessor.validate_samples_and_aggregate_metrics writes the aggregate-metrics sidecar (<dataset>_metrics.json) next to the dataset unconditionally — even when the on-disk sidecar already exists and matches the freshly computed metrics:

maybe_conflicting = self._validate_aggregate_metrics(aggregate_metrics_dict, metrics_fpath)
if maybe_conflicting is not None:
    ...
# always runs, even on a clean match:
with open(metrics_fpath, "w") as f:
    json.dump(aggregate_metrics_dict, f, indent=4)

_validate_aggregate_metrics already returns None both when the sidecar is missing and when it matches, so a matching sidecar still gets truncated and rewritten with identical bytes.

That redundant open(..., "w") breaks a common multi-user setup: when the dataset lives in a shared, read-only directory — e.g. a project-wide pre-staged benchmark workspace bind-mounted into the eval container — a user who doesn't own the pre-staged sidecar hits:

PermissionError: [Errno 13] Permission denied:
  'benchmarks/swebench/data/swebench_verified_benchmark_metrics.json'
  at nemo_gym/train_data_utils.py:670 (validate_samples_and_aggregate_metrics)

The run aborts during data preparation, before any rollout. This surfaced running SWE-bench Verified P/D on a Slurm cluster where the Gym workspace is shared across a project group.

Fix

Skip the write when the sidecar already exists and no conflict was reported. Missing-file and overwrite-on-conflict paths are unchanged.

sidecar state	before	after
missing	write	write
exists & matches	rewrite (redundant)	skip
exists & differs, overwrite_metrics_conflicts=True	overwrite	overwrite
exists & differs, overwrite_metrics_conflicts=False	skip + raise	skip + raise

In the newly skipped case the write was provably a no-op (identical bytes), so on-disk output is unchanged.

Tests

• New test_validate_samples_and_aggregate_metrics_skips_rewrite_when_sidecar_matches: copies the bundled example dataset into a tmp dir, runs once to create the sidecar, makes it read-only, and asserts a second pass neither raises nor rewrites it. Verified this fails without the fix (PermissionError) and passes with it.
• Updated test_validate_samples_and_aggregate_metrics_sanity: its bundled sidecar already matches, so it is no longer rewritten — the write assertion is now [].

pytest tests/unit_tests/test_train_data_utils.py → 22 passed. ruff + pre-commit clean on the changed files.

Note for maintainers

This fixes the redundant-rewrite that breaks the shared-read-only case. The deeper design point — the metrics sidecar is written relative to cwd, which _resolve_under_cwd_or_install documents as needing to be "the user's writable cwd" but which is the shared install root under a bind-mounted workspace — is left as-is. Happy to follow up if you'd prefer the sidecar target to move to an explicit output dir instead.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.