Skip to content

fix(slurm): update dp configs to new shape - #689

Merged
nina-xu merged 1 commit into
mainfrom
nina-xu/fix/slurm-configs
Aug 3, 2026
Merged

fix(slurm): update dp configs to new shape#689
nina-xu merged 1 commit into
mainfrom
nina-xu/fix/slurm-configs

Conversation

@nina-xu

@nina-xu nina-xu commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Following up to #595, needed to update the configs for the DP runs to reflex the new shape for structured_generation, so that slurm runs with those configs can succeed.

Summary by CodeRabbit

  • Configuration
    • Updated generation settings across supported training configurations to use the nested structured-generation option.
    • Structured generation remains enabled with the updated configuration format.

Signed-off-by: Nina Xu <19981858+nina-xu@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 3df8f2b8-2895-43f7-9a3f-ef2106b61b27

📥 Commits

Reviewing files that changed from the base of the PR and between 896002b and 49bc190.

📒 Files selected for processing (3)
  • script/slurm/configs/mistral-dp.yaml
  • script/slurm/configs/smollm3-dp.yaml
  • script/slurm/configs/tinyllama-dp.yaml
📜 Recent review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (4)
**/*.yaml

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

Use two-space indentation, colon-space mappings, SPDX headers, unquoted values unless necessary, and a final newline.

Files:

  • script/slurm/configs/tinyllama-dp.yaml
  • script/slurm/configs/mistral-dp.yaml
  • script/slurm/configs/smollm3-dp.yaml

⚙️ CodeRabbit configuration file

Review YAML for 2-space indentation, SPDX headers when required, unquoted values unless needed, and newline at EOF.

Files:

  • script/slurm/configs/tinyllama-dp.yaml
  • script/slurm/configs/mistral-dp.yaml
  • script/slurm/configs/smollm3-dp.yaml
**/*

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

**/*: Every source file requires the SPDX copyright and license header appropriate to its file format.
End files with a newline, remove trailing whitespace, use one space between sentences, and keep code, comments, and docstrings within 120 characters.

**/*: All contributions must use verified Git commits and DCO sign-off; unsigned or unsigned-off commits cannot be merged.
Branches other than main must follow <author>/<description>, optionally including an issue ID or type; branch names must use lowercase alphanumeric characters and hyphens.
Commits merged to main must follow Conventional Commits, using a valid lowercase type and a description of at most 100 characters.

Files:

  • script/slurm/configs/tinyllama-dp.yaml
  • script/slurm/configs/mistral-dp.yaml
  • script/slurm/configs/smollm3-dp.yaml

⚙️ CodeRabbit configuration file

**/*: Review as a senior maintainer for NeMo Safe Synthesizer. Prioritize issues that can change behavior, break user workflows, weaken privacy guarantees, hide failures, make tests unreliable, or create maintenance risk. Avoid generic style commentary unless it points to a concrete project convention that automated tools will not catch.
Comment only when the finding is actionable and tied to changed code. For each finding, state the impact, the condition that triggers it, and the smallest practical fix. Prefer one precise comment over broad advice. Do not ask for refactors outside the PR scope unless the changed code creates the problem.
Review type guidance: - Potential issue: use for correctness bugs, data loss, privacy leaks,
security risks, broken public APIs, invalid config behavior, missing
validation, hidden failures, nondeterministic tests, or CI breakage.

  • Refactor suggestion: use for local maintainability problems introduced
    by the diff when they have clear future cost, such as duplicated setup,
    unclear boundaries, over-mocking, avoidable complexity, or opaque test
    helpers.
  • Nitpick: avoid in chill mode. Do not emit formatting, import-order,
    wording, or style-only comments unless automated tools cannot catch the
    issue and it affects maintainability.

Severity guidance: - Critical: security/privacy leaks, data loss, training/test/holdout
contamination, or broken release/package/core pipeline execution.

  • Major: incorrect generation/training/evaluation behavior, broken
    CLI/SDK public API, invalid config defaults or validators, or GPU/vLLM
    cleanup and process-isolation bugs likely to fail CI or production
    runs.
  • Minor: localized bugs, missing focused tests for changed behavior, or
    bad test patterns that weaken regression coverage.
  • Trivial: small cleanup with no behavior impact. Usually suppress in
    chill mode.
  • Info: context only. Avoid unless it helps reviewers understand risk.
    Safe-Synthesizer-specific review focus: - Data ...

Files:

  • script/slurm/configs/tinyllama-dp.yaml
  • script/slurm/configs/mistral-dp.yaml
  • script/slurm/configs/smollm3-dp.yaml
**/*.{py,sh,yaml,yml,md}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

All Python, shell, YAML, YML, and Markdown source files require SPDX copyright headers.

Files:

  • script/slurm/configs/tinyllama-dp.yaml
  • script/slurm/configs/mistral-dp.yaml
  • script/slurm/configs/smollm3-dp.yaml
script/**

⚙️ CodeRabbit configuration file

Review standalone scripts for reproducibility and operational safety. Check argument validation, quoting, repo-root detection, environment variables, generated artifacts, external commands, GPU/cluster assumptions, and whether the script should be wired through Makefile or documented in README/docs.

Files:

  • script/slurm/configs/tinyllama-dp.yaml
  • script/slurm/configs/mistral-dp.yaml
  • script/slurm/configs/smollm3-dp.yaml
🔇 Additional comments (3)
script/slurm/configs/mistral-dp.yaml (1)

11-12: LGTM!

script/slurm/configs/smollm3-dp.yaml (1)

11-12: LGTM!

script/slurm/configs/tinyllama-dp.yaml (1)

11-12: LGTM!


Walkthrough

Changes

Structured generation configuration

Layer / File(s) Summary
Update generation settings
script/slurm/configs/mistral-dp.yaml, script/slurm/configs/smollm3-dp.yaml, script/slurm/configs/tinyllama-dp.yaml
The configurations replace use_structured_generation with structured_generation.enabled: true.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested labels: bug

Suggested reviewers: binaryaaron

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the update to Slurm data-parallel configurations.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch nina-xu/fix/slurm-configs

Comment @coderabbitai help to get the list of available commands.

@nina-xu nina-xu changed the title update dp configs to new shape fix: update dp configs to new shape Aug 3, 2026
@nina-xu nina-xu changed the title fix: update dp configs to new shape fix(slurm): update dp configs to new shape Aug 3, 2026
@nina-xu
nina-xu marked this pull request as ready for review August 3, 2026 14:49
@nina-xu
nina-xu requested a review from a team as a code owner August 3, 2026 14:49
@nina-xu
nina-xu requested a review from mckornfield August 3, 2026 14:50
@coderabbitai coderabbitai Bot added the bug Defects in shipped behavior label Aug 3, 2026
@greptile-apps

greptile-apps Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Updates three differential-privacy Slurm configurations to use the current nested generation.structured_generation.enabled configuration shape.

  • Migrates the Mistral DP configuration.
  • Migrates the SmolLM3 DP configuration.
  • Migrates the TinyLlama DP configuration.

Confidence Score: 5/5

The PR appears safe to merge.

The changed YAML structure matches the current typed configuration schema, is preserved by the Slurm-to-CLI loading path, and retains structured generation as enabled without requiring additional fields.

Important Files Changed

Filename Overview
script/slurm/configs/mistral-dp.yaml Replaces the deprecated flat structured-generation flag with the valid nested configuration.
script/slurm/configs/smollm3-dp.yaml Replaces the deprecated flat structured-generation flag with the valid nested configuration.
script/slurm/configs/tinyllama-dp.yaml Replaces the deprecated flat structured-generation flag with the valid nested configuration.

Reviews (1): Last reviewed commit: "update dp configs to new shape" | Re-trigger Greptile

@nina-xu
nina-xu added this pull request to the merge queue Aug 3, 2026
Merged via the queue into main with commit 1e968ec Aug 3, 2026
21 of 25 checks passed
@nina-xu
nina-xu deleted the nina-xu/fix/slurm-configs branch August 3, 2026 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:tests bug Defects in shipped behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants