[debug] option to disable validation by nileshnegi · Pull Request #334 · ROCm/TransferBench

nileshnegi · 2026-07-14T21:30:55Z

Motivation

Adds a performance debug option that disables validation, so only the initialization and actual transfers execute.

Previously, ALWAYS_VALIDATE was a boolean (0 = validate once at the end, 1 = validate every iteration), with no way to skip validation entirely. This makes it difficult to isolate pure transfer/initialization cost from validation overhead.

Technical Details

Adds a "disable" option to ALWAYS_VALIDATE while preserving the existing 0/1 behavior (backward compatible):

ALWAYS_VALIDATE=-1 — validation disabled (init + transfers only)
ALWAYS_VALIDATE=0 — validate once at the end (default, unchanged)
ALWAYS_VALIDATE=1 — validate after each iteration (unchanged)

Changes:

src/header/TransferBench.hpp: per-iteration validation gated on == 1; end-of-run validation gated on == 0; disabled when == -1.
src/client/EnvVars.hpp: updated field comment, help text, and runtime print description for the three modes.
src/client/Presets/SmokeTest.hpp: continues to default to 1 (validate each iteration).
Interactive mode (USE_INTERACTIVE=1): validation is now a separate stage requiring an <Enter> keypress rather than running as part of the transfer stage. It respects ALWAYS_VALIDATE — skipped for -1, <Enter>-gated for 0, and skipped for 1 (which already validates inline each iteration).

Test Plan

Built with make clean && make -j.
Ran ./TransferBench cmdline 1K "1 1 g0 d0 g1" across ALWAYS_VALIDATE={-1,0,1} and USE_INTERACTIVE={0,1}.

Test Result

ALWAYS_VALIDATE=-1 — no validation output; only init + transfers execute (including under USE_INTERACTIVE=1). Confirmed with AMD_LOG_LEVEL that there's no CopyDeviceToHost at the end.
ALWAYS_VALIDATE=0 (default) — single validation at end; in interactive mode it waits for <Enter> before validating and printing PASS/FAIL results.
ALWAYS_VALIDATE=1 — validates each iteration inline; no extra Enter-gated stage in interactive mode.
All transfers reported PASS; builds clean with no new warnings.

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

ALWAYS_VALIDATE now supports three modes: 0 disables validation, 1 validates once at the end (new default), and 2 validates after each iteration. Updates the client help/print text and SmokeTest preset to match the new semantics. Co-authored-by: Cursor <cursoragent@cursor.com>

When USE_INTERACTIVE=1, validation now runs as a distinct stage that requires an <Enter> keypress instead of running as part of the transfer stage. The interactive validation stage respects ALWAYS_VALIDATE: it is skipped when validation is disabled (0), triggered by <Enter> for validate-at-end (1), and skipped for validate-each-iteration (2) since that mode already validates inline every iteration. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Pull request overview

This PR reworks the ALWAYS_VALIDATE setting from a boolean into a 3-mode validation control to better isolate initialization/transfer performance from validation overhead, including updated interactive-mode behavior.

Changes:

Redefined ALWAYS_VALIDATE semantics to: 0=disabled, 1=validate once at end (default), 2=validate each iteration.
Updated backend validation gating logic to match the new semantics and adjusted interactive-mode flow to make validation an explicit, Enter-triggered stage for mode 1.
Updated client defaults/help text and adjusted the SmokeTest preset default to preserve prior validate-each-iteration behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/header/TransferBench.hpp	Implements new validation-mode gating and interactive-mode validation stage behavior.
src/client/EnvVars.hpp	Updates `ALWAYS_VALIDATE` defaults/documentation and runtime display text for the 3 modes.
src/client/Presets/SmokeTest.hpp	Changes SmokeTest’s default `ALWAYS_VALIDATE` to `2` to preserve validate-each-iteration behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Restore the original ALWAYS_VALIDATE semantics and add a disable option: -1 = disable validation entirely (init + transfers only) 0 = validate once after all iterations (default) 1 = validate after each iteration This keeps the previous 0/1 behavior intact while adding -1 as a performance-debug option that skips validation. Interactive mode runs the Enter-gated validation stage only for mode 0; mode 1 validates inline each iteration and mode -1 skips validation. SmokeTest preset now defaults to 1 to preserve its validate-each-iteration behavior. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

- Normalize ALWAYS_VALIDATE on read: warn and clamp values outside {-1, 0, 1} so runtime behavior matches the documented modes, instead of silently disabling validation for out-of-range values. - Avoid validating twice in interactive mode: the interactive stage now records failures into errResults and the end-of-run ValidateAllTransfers is skipped for ranks already validated by that stage. Co-authored-by: Cursor <cursoragent@cursor.com>

scanf("%*c") returns 0 on success and EOF on failure, so the previous != 0 check was effectively an EOF check with a misleading message. Replace with getchar() and report EOF explicitly. Co-authored-by: Cursor <cursoragent@cursor.com>

In interactive mode, the per-transfer validation summary is now displayed for mode 1 (validate each iteration) as well, without requiring an <Enter> keypress since validation already ran inline. Mode 0 keeps its <Enter>-gated validation path and remains the authoritative validation for its rank; mode 1 only displays results and does not re-record errors (already captured inline). Mode -1 still shows nothing. Co-authored-by: Cursor <cursoragent@cursor.com>

Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

gilbertlee-amd

With alwaysValidate=0, we can speed things up even more and skip prepare the dstReference arrays.

For now, let's have:
< 0 = no validation
0 = validate at end (default)

0 = validate per iteration

Per maintainer review, switch to range-based semantics: <0 = disable validation 0 = validate once at end (default) >0 = validate after each iteration Any positive value now enables per-iteration validation (preserving the original truthy behavior), so the explicit clamp/normalize helper is removed. When validation is disabled (<0), the dstReference expected-result arrays are no longer allocated or computed, saving setup time. Co-authored-by: Cursor <cursoragent@cursor.com>

- Print "Validation disabled (ALWAYS_VALIDATE < 0)" in the interactive stage (in place of results) and in Utils::PrintResults for SHOW_DETAILS. - Show the "Transfers complete." prefix for per-iteration mode (>0) in interactive output, matching the validate-at-end (0) path. Co-authored-by: Cursor <cursoragent@cursor.com>

Suppress the PrintResults "validation disabled" message when interactive mode is active, since the interactive stage already prints it. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

- Replace exit(1) on rank-0 EOF at the validation prompt with a broadcast so all ranks abort together (return ERR_FATAL) instead of leaving other ranks hung at the following Barrier. - Correct the interactive-block comment: it performs its own comparison pass to display PASS/FAIL, not merely display. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

nileshnegi and others added 2 commits July 14, 2026 21:20

Copilot AI review requested due to automatic review settings July 14, 2026 21:30

nileshnegi requested a review from a team as a code owner July 14, 2026 21:30

Copilot started reviewing on behalf of nileshnegi July 14, 2026 21:31 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread src/client/EnvVars.hpp Outdated

Comment thread src/header/TransferBench.hpp Outdated

Copilot AI review requested due to automatic review settings July 14, 2026 21:36

Copilot started reviewing on behalf of nileshnegi July 14, 2026 21:37 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread src/header/TransferBench.hpp Outdated

Comment thread src/client/EnvVars.hpp Outdated

Comment thread src/header/TransferBench.hpp Outdated

nileshnegi and others added 4 commits July 14, 2026 21:44

Condense verbose comments introduced in this branch

50f9f26

Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot AI review requested due to automatic review settings July 14, 2026 21:54

Copilot started reviewing on behalf of nileshnegi July 14, 2026 21:54 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread src/header/TransferBench.hpp Outdated

Comment thread src/header/TransferBench.hpp Outdated

Comment thread src/header/TransferBench.hpp Outdated

gilbertlee-amd requested changes Jul 14, 2026

View reviewed changes

nileshnegi and others added 3 commits July 14, 2026 22:07

Avoid duplicate validation-disabled message in interactive mode

92a2219

Suppress the PrintResults "validation disabled" message when interactive mode is active, since the interactive stage already prints it. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot AI review requested due to automatic review settings July 14, 2026 22:18

Copilot started reviewing on behalf of nileshnegi July 14, 2026 22:19 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread src/header/TransferBench.hpp Outdated

Comment thread src/header/TransferBench.hpp

Comment thread src/header/TransferBench.hpp

Copilot AI review requested due to automatic review settings July 14, 2026 22:35

Copilot started reviewing on behalf of nileshnegi July 14, 2026 22:36 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread src/header/TransferBench.hpp

gilbertlee-amd approved these changes Jul 15, 2026

View reviewed changes

nileshnegi merged commit ec8d0d2 into candidate Jul 15, 2026
11 checks passed

nileshnegi deleted the users/nileshnegi/feat/option-disable-val branch July 15, 2026 05:47

Uh oh!

Conversation

nileshnegi commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Technical Details

Test Plan

Test Result

Submission Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gilbertlee-amd left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nileshnegi commented Jul 14, 2026 •

edited

Loading