Skip to content

Add --randomize-configs flag to runbms for noise/spike analysis#171

Merged
caizixian merged 6 commits intomasterfrom
copilot/fix-0e469fbf-2daf-4089-a6d7-7ba2857095fe
Sep 29, 2025
Merged

Add --randomize-configs flag to runbms for noise/spike analysis#171
caizixian merged 6 commits intomasterfrom
copilot/fix-0e469fbf-2daf-4089-a6d7-7ba2857095fe

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Sep 29, 2025

  • Explore repository structure and understand current runbms implementation
  • Identify where configs are processed (line 438 in runbms.py, passed to run_one_benchmark)
  • Understand test infrastructure and build process
  • Add --randomize-configs flag to runbms argument parser
  • Import random module and implement config shuffling when flag is set
  • Ensure randomization preserves original config list for logging/metadata
  • Add tests to verify argument parsing functionality
  • Add test to verify randomization logic works correctly
  • Verify all existing tests continue to pass
  • Verify linting passes (mypy)
  • Manual verification of help text and command line interface
  • Format code with black per maintainer request
  • Verify all tests still pass after formatting (44/44 tests pass)
  • Update changelog and documentation to mark flag as preview

Implementation Summary:

This PR implements support for randomizing the order of configs for the runbms subcommand as requested in the issue. The feature helps identify whether performance spikes/noise are related to system issues (like I/O) or specific configurations.

Key Changes:

  1. New CLI flag: Added --randomize-configs argument to the runbms command
  2. Randomization Logic: Modified the config iteration in run_one_benchmark() to shuffle config indices when the flag is set
  3. Preserves Logging: Original config indices are maintained for proper logging and plugin callbacks
  4. Per-invocation Randomization: Configs are randomized separately for each invocation, maximizing the benefits for noise detection

Documentation Updates:

  • Added entry to changelog.md under "Unreleased > Added > Commands" section
  • Updated runbms.md command reference documentation with usage syntax and flag description
  • Marked the flag as "(preview ⚠️)" following the existing pattern for other preview flags

Technical Details:

  • Follows existing code patterns using global variables for configuration options
  • Uses random.shuffle() on config indices rather than configs directly to preserve index-based logging
  • Randomization occurs per invocation (inner loop) rather than per benchmark for maximum effectiveness
  • All existing functionality preserved - flag defaults to False for backward compatibility
  • Code formatted with black per maintainer requirements

Testing:

  • Added argument parser test to verify flag is properly recognized
  • Added logic test to verify randomization works as expected
  • All existing tests (44 total) continue to pass
  • Type checking passes with mypy
  • Code formatting passes with black
Original prompt

This section details on the original issue you should resolve

<issue_title>Support randomizing the order of configs for runbms subcommand</issue_title>
<issue_description>This is helpful for figuring out noises/spikes are related to a system issue (e.g. IO) or it is related to one particular config.</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes #170

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Copilot AI and others added 2 commits September 29, 2025 05:05
Co-authored-by: caizixian <2891235+caizixian@users.noreply.github.com>
Co-authored-by: caizixian <2891235+caizixian@users.noreply.github.com>
Copilot AI changed the title [WIP] Support randomizing the order of configs for runbms subcommand Add --randomize-configs flag to runbms for noise/spike analysis Sep 29, 2025
Copilot AI requested a review from caizixian September 29, 2025 05:13
@caizixian caizixian requested review from k-sareen and wks September 29, 2025 05:15
@caizixian
Copy link
Copy Markdown
Member

@copilot reformat the code with black, and check whether all tests pass by pytest. Check the pyproject file on what dependencies you need to install.

Copy link
Copy Markdown
Contributor

@k-sareen k-sareen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

Co-authored-by: caizixian <2891235+caizixian@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Sep 29, 2025

@copilot reformat the code with black, and check whether all tests pass by pytest. Check the pyproject file on what dependencies you need to install.

Formatted the code with black and verified all tests pass. The formatting changes were applied to src/running/command/runbms.py and tests/test_runbms.py with improvements to spacing and string formatting. All 44 tests continue to pass after formatting. Changes committed in f8eaaa2.

@caizixian
Copy link
Copy Markdown
Member

@copilot Please update the changelog and the command reference doc (mark it as preview just like other preview flags).

Co-authored-by: caizixian <2891235+caizixian@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Sep 29, 2025

@copilot Please update the changelog and the command reference doc (mark it as preview just like other preview flags).

Updated the changelog and command reference documentation. Added the --randomize-configs flag to the unreleased section of changelog.md and updated the runbms.md documentation with usage syntax and description, marking it as "(preview ⚠️)" following the existing pattern. Changes committed in 3b9b8d7.

@caizixian caizixian marked this pull request as ready for review September 29, 2025 05:55
@caizixian caizixian merged commit 939de1b into master Sep 29, 2025
14 checks passed
@caizixian caizixian deleted the copilot/fix-0e469fbf-2daf-4089-a6d7-7ba2857095fe branch September 29, 2025 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support randomizing the order of configs for runbms subcommand

3 participants