Support multiple verbosity in dump comparator#19684
Merged
fzyzcjy merged 618 commits intosgl-project:mainfrom Mar 2, 2026
Merged
Support multiple verbosity in dump comparator#19684fzyzcjy merged 618 commits intosgl-project:mainfrom
fzyzcjy merged 618 commits intosgl-project:mainfrom
Conversation
- test_dp_alias_absent_group_noop: single rank with dp_size=1, verifies // syntax doesn't break comparison - test_dp_alias_via_override_dims: uses moe_dp_rank/moe_dp_size fields so override-dims with // dp:=moe_dp triggers real alias-based filtering
parse_dims() now returns DimsSpec (dims + dp_group_alias) instead of list[DimSpec]. This removes the separate extract_dp_group_alias() public API and keeps dp group alias extraction as an internal detail of the parsing step.
# is more natural as an annotation/pragma marker and avoids ambiguity with URL fragments or division operators.
… control Instead of all-or-nothing --forbid-skip, --allow-skip-pattern accepts a regex: tensor names matching the pattern are allowed to skip (e.g. core auto-dump fields like positions/seq_lens that lack dims metadata at TP>1). Default '.*' allows all skips; '^$' forbids all (equivalent to old --forbid-skip).
Replace scattered print_record() calls with a centralized report_sink singleton that tees output to both stdout and an auto-generated JSONL report file. This eliminates output_format parameter threading through the call chain. - Add ReportSink class with configure/add/close lifecycle - Add --report-path and --no-report CLI arguments - Default report path: <target-path>/comparator_report.jsonl - Remove output_format from emit_display_records, _consume_comparison_records, WarningSink - Add TestReportOutput test class and conftest autouse fixture for isolation
- Report path now printed via report_sink.add(ReportPathRecord(...)) so it respects json/text output format - --no-report removed; pass --report-path '' to disable instead
…_seq_lens_only `&` and `-` are same-precedence left-associative so behavior was correct, but explicit parentheses make the intent unambiguous.
Cover: no plugin, empty cp_sharded_names, sglang seq_lens extraction, megatron cu_seqlens_q diff extraction, multi-step, and missing tensor.
Verifies that concat mode correctly loads thd_seq_lens and performs zigzag→natural reorder for Megatron-format CP=2 THD tensors.
…nly to concat_steps package - Rename token_aligner/concat.py → token_aligner/concat_steps/ package - Rename execute_token_aligner_concat → execute_token_aligner_concat_steps - Move load_thd_seq_lens_only from smart/aux_loader.py to concat_steps/thd_seq_lens_loader.py so concat-specific code no longer lives inside the smart subpackage - Update CLI choices, defaults, and all string literals from "concat" to "concat_steps" - Update all imports and test files accordingly
The concat_steps/__init__.py eagerly importing thd_seq_lens_loader created a circular import chain through smart/aux_loader.py → entrypoint/executor.py. Import from the submodule path instead.
- Add TracedAlignerPlan wrapper types with ShapeSnapshot tracking - Extract ReportSink to report_sink.py, add verbosity field - Add BundleFileInfo/BundleSideInfo/ShapeSnapshot types to output_types - Change executor return values to NamedTuples (StepPlansResult, SubPlansResult) - Refactor TensorComparisonRecord: aligner_plan -> traced_plan, add raw_bundle_info - Make extract_parallel_info and PARALLEL_INFO_KEYS public in display.py - Add --verbosity CLI parameter to entrypoint - Add testing_helpers.py with shared test factories - Add to_rich() / _format_rich_body() stubs to _OutputRecord
Extract output_formatter.py for record-level rendering delegation, add Rich markup formatters in tensor_comparator/formatter.py, and wire up display.py with _render_polars_as_rich_table. All Rich functions hardcoded to normal mode (no verbosity params).
Add Rich body tests for ConfigRecord, SkipRecord, NonTensorRecord, SummaryRecord, and log attachment (to_rich). Add Rich table tests for RankInfoRecord and InputIdsRecord. Add comprehensive snapshot tests for format_comparison_rich, _format_bundle_section, _format_plan_section_rich, _format_stats_rich, and _format_abs_diff_percentiles_rich.
Thread verbosity parameter through the full rendering pipeline: report_sink → output_types.to_rich() → output_formatter → formatter. Add _format_comparison_minimal for single-line output, verbose branches for _format_bundle_section/_format_stats_rich, and show_detail logic (verbose OR failed) for samples/checks/percentiles. Move output_formatter imports to top-level in output_types.py.
Move TestFormatComparisonRichMinimal before TestFormatComparisonRichNormal, update Normal docstring to include verbosity qualifier.
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the dump comparator's capabilities by introducing a robust verbosity system, a redesigned dimension specification parser, and detailed execution tracing for alignment plans. These changes aim to provide users with more insightful and configurable debugging information, making it easier to identify and resolve discrepancies in tensor outputs across different parallelization strategies. The refactor also includes a centralized logging mechanism and support for command-line presets, streamlining the comparison workflow. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-executed refactoring of the dump comparator tool. Key improvements include support for multiple verbosity levels, a more powerful and expressive dimension specification syntax (including fused dimensions and comments), and a much cleaner code organization with better separation of concerns. The move from a simple warning_sink to a more structured log_sink with error/info levels is also a great enhancement. Overall, these changes make the tool more powerful, user-friendly, and maintainable. I have one minor suggestion regarding a regression in the text output format.
| "[p95] 1.5000 vs 1.5000 (diff: 0.0000)\n" | ||
| "[p99] 1.8000 vs 1.8000 (diff: 0.0000)\n" | ||
| "✅ rel_diff=0.0001\t✅ max_abs_diff=0.0005\t✅ mean_abs_diff=0.0002\n" | ||
| "✅ rel_diff=0.0001\tmax_abs_diff=0.0005\tmean_abs_diff=0.0002\n" |
Contributor
There was a problem hiding this comment.
This test is being updated to reflect the removal of pass/fail markers (✅/❌) for max_abs_diff and mean_abs_diff in the text-only output format. This seems to be a regression, as these markers provided useful at-a-glance information. While the new rich output is the main focus, the simpler text format shouldn't lose informativeness. Please consider restoring the markers in the implementation and updating this test accordingly.
Suggested change
| "✅ rel_diff=0.0001\tmax_abs_diff=0.0005\tmean_abs_diff=0.0002\n" | |
| "✅ rel_diff=0.0001\t✅ max_abs_diff=0.0005\t✅ mean_abs_diff=0.0002\n" |
Kangyan-Zhou
pushed a commit
to Kangyan-Zhou/sglang
that referenced
this pull request
Mar 4, 2026
magicYang1573
pushed a commit
to magicYang1573/sglang
that referenced
this pull request
Mar 9, 2026
Wangzheee
pushed a commit
to Wangzheee/sglang
that referenced
this pull request
Mar 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Modifications
Accuracy Tests
Benchmarking and Profiling
Checklist
Review Process
/tag-run-ci-label,/rerun-failed-ci,/tag-and-rerun-ci