[malay-nagda]

What does this PR do ?

Adds an alternative to --custom_env_vars for env vars with comma separated values as part of a single var.

Changelog

slurm_args.add_argument(
        "-E",
        "--env",
        action="append",
        type=parse_kv,
        metavar="KEY=VALUE",
        help="Set environment variable (repeatable arg). This is an alternative to --custom_env_vars \
        (--custom_env_vars is preferred for most cases). Example: -E var1=value1,value2 -E var2=value3",
    )

GitHub Actions CI

See the CI sectionin the Contributing doc for how to trigger the CI. A Nvidia developer will need to approve and trigger the CI for external contributors.

Before your PR is "Ready for review"

Pre checks:

• Make sure you read and followed Contributor guidelines
• Did you write any new necessary tests?
• Did you add or update any necessary documentation?
• Does the PR affect components that are optional to install? (Ex: Numba, Pynini, Apex etc)
  • Reviewer: Does the PR have correct import guards for all optional libraries?

If you haven't finished some of the above items you can still open "Draft" PR.

Additional Information

• Related to # (issue)

Summary by CodeRabbit

Release Notes

• New Features

  • Added a new -E/--env command-line argument to specify environment variables as repeatable KEY=VALUE pairs, providing an alternative to existing configuration methods.

• Documentation

  • Updated documentation with usage examples for the new environment variable argument.

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR introduces a new repeatable -E/--env CLI argument for the performance testing script as an alternative to --custom_env_vars. Changes include documentation updates, a new parse_kv() validation function in the argument parser, and logic to merge CLI-provided environment variables into the existing configuration.

Changes

Cohort / File(s)	Summary
Environment Variable CLI Feature scripts/performance/README.md, scripts/performance/argument_parser.py, scripts/performance/setup_experiment.py	Adds new -E/--env repeatable argument with KEY=VALUE validation via new parse_kv() function. Integrates parsed environment variables by merging them into custom_env_vars dictionary in setup flow.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title mentions 'comma separated' but the PR actually implements repeatable -E/--env arguments as an alternative to comma-separated values, not comma-separated values themselves.	Update the title to reflect the actual change, such as 'Add repeatable -E/--env CLI argument for setting environment variables' or 'Introduce alternative -E flag for environment variable configuration'.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Test Results For Major Changes	✅ Passed	PR adds a new CLI argument option (-E/--env) as an alternative input method for environment variables with only 30 lines of code across three files and no changes to training logic, computation, algorithms, or core execution behavior.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch malay/multiple_env_vars

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Tip

CodeRabbit can generate a title for your PR based on the changes.

Add @coderabbitai title placeholder anywhere in the title of your PR and CodeRabbit will replace it with a title based on the changes in the PR. You can change the placeholder by changing the reviews.auto_title_placeholder setting.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/performance/argument_parser.py`:
- Around line 446-447: Update the help text for the --env / -E argument to show
repeatable-flag usage instead of comma-separated values: locate the add_argument
call that defines "--env" (the string "--env" or "-E" in argument_parser.py) and
replace the example portion with a repeatable example such as "-E var1=value1 -E
var1=value2 -E var2=value3" (and mention it is repeatable) so the help clearly
demonstrates using the flag multiple times rather than using commas.

In `@scripts/performance/README.md`:
- Line 157: Fix the typo and clarify the repeatable flag usage for the -E/--env
option: remove the stray trailing quote and replace the comma-based example with
a repeatable-flag example showing separate -E occurrences (e.g., `-E var1=value1
-E var1=value2 -E var2=value3`) and mention that this is an alternative to
`--custom_env_vars`. Update the README line referencing `-E/--env` and
`--custom_env_vars` accordingly.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 05f499ef-43b2-4817-8c3f-1422aa97e3c4

📥 Commits

Reviewing files that changed from the base of the PR and between 57d10ad and 8a7c6fa.

📒 Files selected for processing (3)

• scripts/performance/README.md
• scripts/performance/argument_parser.py
• scripts/performance/setup_experiment.py

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Clarify the --env help example to match repeatable usage.

The current example still looks comma-oriented. Prefer a pure repeatable-flag example for unambiguous UX.

✏️ Suggested help-text tweak

-        help="Set environment variable (repeatable arg). This is an alternative to --custom_env_vars \
-        (--custom_env_vars is preferred for most cases). Example: -E var1=value1,value2 -E var2=value3",
+        help="Set environment variable (repeatable arg). This is an alternative to --custom_env_vars \
+        (--custom_env_vars is preferred for most cases). Example: -E var1=value1 -E var2=value2",

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	help="Set environment variable (repeatable arg). This is an alternative to --custom_env_vars \
	(--custom_env_vars is preferred for most cases). Example: -E var1=value1,value2 -E var2=value3",
	help="Set environment variable (repeatable arg). This is an alternative to --custom_env_vars \
	(--custom_env_vars is preferred for most cases). Example: -E var1=value1 -E var2=value2",

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@scripts/performance/argument_parser.py` around lines 446 - 447, Update the
help text for the --env / -E argument to show repeatable-flag usage instead of
comma-separated values: locate the add_argument call that defines "--env" (the
string "--env" or "-E" in argument_parser.py) and replace the example portion
with a repeatable example such as "-E var1=value1 -E var1=value2 -E var2=value3"
(and mention it is repeatable) so the help clearly demonstrates using the flag
multiple times rather than using commas.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Fix the -E/--env example typo and command format.

The example has a stray trailing quote and is easy to misread as comma-based input rather than repeatable flags.

✏️ Suggested doc fix

-- `-E/--env`: Set environment variable (repeatable arg). This is an alternative to `--custom_env_vars`. (`--custom_env_vars` is preferred for most cases). Example: `-E var1=value1,value2 -E var2=value3"`.
+- `-E/--env`: Set environment variable (repeatable arg). This is an alternative to `--custom_env_vars` (`--custom_env_vars` is preferred for most cases). Example: `-E var1=value1 -E var2=value2`.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@scripts/performance/README.md` at line 157, Fix the typo and clarify the
repeatable flag usage for the -E/--env option: remove the stray trailing quote
and replace the comma-based example with a repeatable-flag example showing
separate -E occurrences (e.g., `-E var1=value1 -E var1=value2 -E var2=value3`)
and mention that this is an alternative to `--custom_env_vars`. Update the
README line referencing `-E/--env` and `--custom_env_vars` accordingly.