Skip to content

Conversation

@chen2021673
Copy link
Contributor

@chen2021673 chen2021673 commented Feb 4, 2026

What

This PR refactors the internal NamedModules API to use a more ordered and explicit return type, improves name lookup precision, and adds a public wrapper for convenience.

Why

  • The previous private NamedModules return type was not ordered and harder to consume reliably.
  • Callers needed a dedicated interface to retrieve named modules with options like recursion and duplicate removal.
  • Centralizing the module→name mapping simplifies downstream logic and makes name-based checks more precise.

Changes

  • Changed private NamedModules() to return an ordered std::vector<std::pair<...>> instead of the old type.
  • Introduced BuildNameMap() utility to build a stable module-to-name mapping.

Result

  • More predictable API for named module enumeration.
  • Greater clarity and maintainability in module name handling.

@kilinchange
Copy link
Collaborator

kilinchange commented Feb 8, 2026

麻烦运行一下测试脚本,并贴出 loss/tps 测试结果的截图。

@chen2021673
Copy link
Contributor Author

chen2021673 commented Feb 9, 2026

麻烦运行一下测试脚本,并贴出 loss/tps 测试结果的截图。

baseline:20251223/feature/tp-pp-split-stream
loss 测试结果:
image
image

tps 测试结果:
image
image

@chen2021673 chen2021673 force-pushed the refactor_named_module branch from de6cce7 to 7ad6863 Compare February 9, 2026 05:40
- Rename private NamedModules() to use odered std::vector<std::pair> return type
- Add public named_modules() wrapper with recurse/remove_duplicate params
- Add BuildNameMap() to create module->name map for precision checking

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@chen2021673 chen2021673 force-pushed the refactor_named_module branch from 7ad6863 to b97bf82 Compare February 9, 2026 05:47
@kilinchange kilinchange merged commit cbc364e into master Feb 9, 2026
2 checks passed
@kilinchange kilinchange deleted the refactor_named_module branch February 9, 2026 05:51
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.

2 participants