Skip to content

Refactor: rename dump-tensor public surface to dump-args (#1075)#1143

Merged
ChaoZheng109 merged 1 commit into
hw-native-sys:mainfrom
doraemonmj:rename-dump-args-1075
Jun 25, 2026
Merged

Refactor: rename dump-tensor public surface to dump-args (#1075)#1143
ChaoZheng109 merged 1 commit into
hw-native-sys:mainfrom
doraemonmj:rename-dump-args-1075

Conversation

@doraemonmj

Copy link
Copy Markdown
Contributor

What

Finishes the public-surface rename started in #1072. That PR renamed the
--dump-args CLI flag, the args_dump/ artifact directory, the manifest,
and the dump_viewer tool, but several user-facing surfaces still named the
legacy dump tensor feature. The feature semantics are per-task argument
dump, so this completes the rename across tests, CI, and docs (issue #1075).

Changes

  • dfx smoke test: dfx/tensor_dump/test_tensor_dump.py
    dfx/args_dump/test_args_dump.py; class TestTensorDumpTestArgsDump
    (and its output-label string).
  • CI: both ci.yml jobs — step name tensor_dump smokeargs_dump smoke
    and the test path.
  • Docs: docs/dfx/tensor-dump.mdargs-dump.md with all inbound links
    fixed (tools/README.md, src/{a2a3,a5}/docs/runtimes.md,
    profiling-framework.md).
  • Stale comments/README naming the renamed public surface: the old
    --dump-tensor flag (task_interface.cpp, vector_add/README.md) and the
    old tensor_dump/ output dir (call_config.h, device_runner_base.h).

Out of scope (intentionally untouched)

  • Internal C++ implementation names (CallConfig::enable_dump_tensor,
    TensorDumpCollector, etc.) — not part of the public surface.
  • host_build_graph/dump_tensor examples — they genuinely demonstrate the
    tensor-info dump APIs (set_tensor_info_to_task), not the unified args-dump
    feature, so their name is correct.

No behavior change — renames and comment/doc fixes only.

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e10944f6-2395-49ff-b6de-ff8b0bb62b23

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR updates the A2A3 DFX smoke path to use args_dump instead of tensor_dump, renames the matching smoke test class and lookup label, and refreshes related docs, README references, and inline comments to use args-dump terminology.

Changes

Args-dump naming rollout

Layer / File(s) Summary
Smoke workflow and test rename
\.github/workflows/ci.yml, tests/st/a2a3/tensormap_and_ringbuffer/dfx/args_dump/test_args_dump.py
The A2A3 simulation and onboard smoke jobs run the args_dump test target, and the smoke test class and output-label lookup use TestArgsDump.
User-facing docs references
docs/profiling-framework.md, simpler_setup/tools/README.md, src/a2a3/docs/runtimes.md, src/a5/docs/runtimes.md, examples/workers/l2/vector_add/README.md
The profiling docs, runtime docs, tool README, and example README replace tensor-dump references with args-dump references and add dump_viewer.
Internal comments and bindings
src/common/platform/onboard/host/device_runner_base.h, src/common/task_interface/call_config.h, python/bindings/task_interface.cpp
Inline comments update the diagnostic dump directory names and related flag text from tensor_dump/--dump-tensor to args_dump/--dump-args.

🎯 2 (Simple) | ⏱️ ~10 minutes

🐰 I hop from tensor to args with glee,
and sprinkle docs on every tree.
The smoke test sings, the paths align,
my whiskers twitch—everything’s fine!
Hop hop, new names, bright as can be.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main change: renaming the public dump-tensor surface to dump-args.
Description check ✅ Passed The description matches the changeset and explains the rename across tests, CI, docs, and comments.
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.

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


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

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

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request renames the 'tensor dump' feature to 'args dump' across various documentation files, code comments, examples, and test cases to ensure consistent terminology. There are no review comments, so I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@doraemonmj doraemonmj force-pushed the rename-dump-args-1075 branch from 5604485 to a053568 Compare June 25, 2026 02:10
@vegetabledoww

Copy link
Copy Markdown
Contributor

PR1143 rename remaining items (exclude host_build_graph example rename)

Scope:

  • Only list pure rename/text cleanup items for dump tensor -> dump args.
  • Exclude host_build_graph dump_tensor example directory/file rename for now.
  • Exclude non-rename issues such as broken markdown relative links or stray typo text.
  1. simpler_setup/tools/init.py

Current text:
dump_viewer : inspect tensor dumps

Expected rename:
dump_viewer : inspect args dumps

Reason:
This is user/developer-facing CLI tool description. It should use the new args dump terminology.

  1. docs/dfx/scope-stats.md

Current text in the feature comparison table:
tensor dump | platform only | all runtimes | dumps tensor data

Expected rename:
args dump | platform only | all runtimes | dumps argument data

Reason:
This is documentation-facing terminology. The feature should be referred to as args dump after PR1075.

  1. a2a3 tensormap_and_ringbuffer ST comments introduced by PR1100

These are comments only. They were introduced by PR1100 merge commit:
8b34712
PR #1100: Fix: complete incore tensor signatures in tensormap_and_ringbuffer ST

Current wording pattern:
tensor dump's ...
tensor dump (which ...)

Expected rename:
args dump's ...
args dump (which ...)

Files/lines currently containing the old wording:
tests/st/a2a3/tensormap_and_ringbuffer/fanin_lookup_perf/test_fanin_lookup_perf.py:43
tests/st/a2a3/tensormap_and_ringbuffer/spmd_batch_dispatch_oob/test_spmd_batch_dispatch_oob.py:50
tests/st/a2a3/tensormap_and_ringbuffer/dummy_task/test_dummy_task.py:61
tests/st/a2a3/tensormap_and_ringbuffer/spmd_multiblock_aiv/test_spmd_multiblock_aiv.py:43
tests/st/a2a3/tensormap_and_ringbuffer/spmd_sync_start/test_spmd_sync_start.py:41
tests/st/a2a3/tensormap_and_ringbuffer/spmd_paged_attention/test_spmd_paged_attention.py:41
tests/st/a2a3/tensormap_and_ringbuffer/dfx/dep_gen/test_dep_gen_chain.py:68
tests/st/a2a3/tensormap_and_ringbuffer/spmd_multiblock_mix/test_spmd_multiblock_mix.py:45
tests/st/a2a3/tensormap_and_ringbuffer/spmd_sync_start_aiv/test_spmd_sync_start_aiv.py:40
tests/st/a2a3/tensormap_and_ringbuffer/spmd_sync_start_edge/test_spmd_sync_start_edge.py:41
tests/st/a2a3/tensormap_and_ringbuffer/spmd_sync_start_stress/test_spmd_sync_start_stress.py:66
tests/st/a2a3/tensormap_and_ringbuffer/spmd_basic/test_spmd_basic.py:49
tests/st/a2a3/tensormap_and_ringbuffer/spmd_starvation/test_spmd_starvation.py:66

Reason:
These comments are not behavior-affecting and were introduced after PR1075 by PR1100, but they are still developer-visible old terminology on current main. If PR1143 is intended to clean up current rename residue, they are safe, low-risk text-only replacements.

…ys#1075)

PR hw-native-sys#1072 renamed the --dump-args CLI flag, the args_dump/ artifact
directory, the manifest, and the dump_viewer tool, but left several
user-facing surfaces still naming the legacy "dump tensor" feature.
The feature semantics are per-task argument dump, so finish the rename:

- Rename the dfx smoke test dir + file tensor_dump/test_tensor_dump.py
  -> args_dump/test_args_dump.py and class TestTensorDump -> TestArgsDump
  (plus its output-label string)
- Update both ci.yml jobs: step name and test path
- Rename docs/dfx/tensor-dump.md -> args-dump.md and fix all inbound
  links (tools/README, src/{a2a3,a5}/docs/runtimes.md, profiling-framework)
- Fix stale comments/README naming the renamed public surface: the old
  --dump-tensor flag (task_interface.cpp, vector_add README) and the old
  tensor_dump/ output dir (call_config.h, device_runner_base.h)

Internal C++ implementation names (CallConfig::enable_dump_tensor,
TensorDumpCollector, etc.) are intentionally left unchanged — out of
scope for a public-surface rename. The host_build_graph dump_tensor
examples are also untouched: they genuinely demonstrate the tensor-info
dump APIs (set_tensor_info_to_task), not the unified args-dump feature.
@doraemonmj doraemonmj force-pushed the rename-dump-args-1075 branch from a053568 to 09dab94 Compare June 25, 2026 03:36
@doraemonmj

Copy link
Copy Markdown
Contributor Author

PR1143 rename remaining items (exclude host_build_graph example rename)

Scope:

  • Only list pure rename/text cleanup items for dump tensor -> dump args.
  • Exclude host_build_graph dump_tensor example directory/file rename for now.
  • Exclude non-rename issues such as broken markdown relative links or stray typo text.
  1. simpler_setup/tools/init.py

Current text: dump_viewer : inspect tensor dumps

Expected rename: dump_viewer : inspect args dumps

Reason: This is user/developer-facing CLI tool description. It should use the new args dump terminology.

  1. docs/dfx/scope-stats.md

Current text in the feature comparison table: tensor dump | platform only | all runtimes | dumps tensor data

Expected rename: args dump | platform only | all runtimes | dumps argument data

Reason: This is documentation-facing terminology. The feature should be referred to as args dump after PR1075.

  1. a2a3 tensormap_and_ringbuffer ST comments introduced by PR1100

These are comments only. They were introduced by PR1100 merge commit: 8b34712 PR #1100: Fix: complete incore tensor signatures in tensormap_and_ringbuffer ST

Current wording pattern: tensor dump's ... tensor dump (which ...)

Expected rename: args dump's ... args dump (which ...)

Files/lines currently containing the old wording: tests/st/a2a3/tensormap_and_ringbuffer/fanin_lookup_perf/test_fanin_lookup_perf.py:43 tests/st/a2a3/tensormap_and_ringbuffer/spmd_batch_dispatch_oob/test_spmd_batch_dispatch_oob.py:50 tests/st/a2a3/tensormap_and_ringbuffer/dummy_task/test_dummy_task.py:61 tests/st/a2a3/tensormap_and_ringbuffer/spmd_multiblock_aiv/test_spmd_multiblock_aiv.py:43 tests/st/a2a3/tensormap_and_ringbuffer/spmd_sync_start/test_spmd_sync_start.py:41 tests/st/a2a3/tensormap_and_ringbuffer/spmd_paged_attention/test_spmd_paged_attention.py:41 tests/st/a2a3/tensormap_and_ringbuffer/dfx/dep_gen/test_dep_gen_chain.py:68 tests/st/a2a3/tensormap_and_ringbuffer/spmd_multiblock_mix/test_spmd_multiblock_mix.py:45 tests/st/a2a3/tensormap_and_ringbuffer/spmd_sync_start_aiv/test_spmd_sync_start_aiv.py:40 tests/st/a2a3/tensormap_and_ringbuffer/spmd_sync_start_edge/test_spmd_sync_start_edge.py:41 tests/st/a2a3/tensormap_and_ringbuffer/spmd_sync_start_stress/test_spmd_sync_start_stress.py:66 tests/st/a2a3/tensormap_and_ringbuffer/spmd_basic/test_spmd_basic.py:49 tests/st/a2a3/tensormap_and_ringbuffer/spmd_starvation/test_spmd_starvation.py:66

Reason: These comments are not behavior-affecting and were introduced after PR1075 by PR1100, but they are still developer-visible old terminology on current main. If PR1143 is intended to clean up current rename residue, they are safe, low-risk text-only replacements.

已修改

@doraemonmj

Copy link
Copy Markdown
Contributor Author

@vegetabledoww 已修改

@ChaoZheng109 ChaoZheng109 merged commit d7bded0 into hw-native-sys:main Jun 25, 2026
16 checks passed
@doraemonmj doraemonmj deleted the rename-dump-args-1075 branch June 25, 2026 06:35
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.

3 participants