docs(rfc): move model-topology logic from C++ to Python (design-only) - #133
Merged
Conversation
Scoped v2 extraction: Python computes is_sparse/corr_id, passed via a new set_topology_v2 binding; C++ keeps all memory/IO and the uint32_t TensorID runtime lookups. Momus-approved, design-only pre-implementation RFC. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Extract InitializeTopology's pipeline construction into BuildTopologyFromSpecs and add InitializeTopologyV2 that reads is_sparse and corr_id from the caller instead of deriving them from loop indices. v1 path preserved. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Forward the enriched (name, is_sparse, groups, corr_ids) topology from Python to InitializeTopologyV2; set_topology (v1) kept intact. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
get_topology results are lowered to the v2 tuple and sent via set_topology_v2, mirroring the C++ bit-packing exactly. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Remove the (name, groups) InitializeTopology/SetTopology path and its binding; the (name, is_sparse, groups, corr_ids) form is now the only set_topology. The signature change spans header+impl+binding so it lands atomically. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Extract is_sparse/corr_id lowering into moe_infinity/utils/topology.build_topology_specs and call the single set_topology; drops the inline _build_topology_v2 method. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Covers dense/sparse flag, structural corr_id packing, last-stage 0xFFFFFFFF marker, and equality with an independent reference. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
drunkcoding
marked this pull request as ready for review
August 5, 2026 14:12
added 2 commits
August 9, 2026 11:15
Signed-off-by: drunkcoding <leyang.xue@ed.ac.uk>
Signed-off-by: drunkcoding <leyang.xue@ed.ac.uk>
Contributor
Author
|
Scope decision required for d0ffd65: A. Accept v1 removal: smaller surface, but breaking API and no rollback/parity oracle. Recommendation and implemented default: B, matching RFC T9. Production uses set_topology_v2; set_topology remains an unused tested fallback. |
added 2 commits
August 9, 2026 11:16
Signed-off-by: drunkcoding <leyang.xue@ed.ac.uk>
Signed-off-by: drunkcoding <leyang.xue@ed.ac.uk>
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.
RFC (design-only, pre-implementation)
This is a draft, design-only PR opened before implementation, per request. It contains only the plan doc at
docs/rfcs/topology-logic-python-extraction.md. No source code is changed yet.Summary
Reduce the amount of model-structure semantics computed inside the C++ hot-path module
core/model/model_topology.cpp, moving it into Python (where the topology is already built inget_topology()), without moving any performance-critical runtime lookup or any memory/IO work.Scoped extraction: Python computes
is_sparse+ packedcorr_idper node and passes them through a newset_topology_v2binding; C++ stops re-deriving those from loop indices but keeps everything else —byte_sizefromkTensorIndex, GPU device placement,kHostMemoryPoolallocation, partition reads,SetDevice, and theuint32_t TensorID -> Node*runtime lookups (GetNodeFromTensorID,GetNodeBodyFromCorrID).Why this scope (honest caveat)
Analysis shows the movable surface is small and coupled to the memory-allocation loop it lives in.
InitializeTopologyinterleaves trivial index math (movable) with memory/IO work (must stay C++). The value is separation of concerns / single source of truth for model semantics, not performance. Section 8 of the plan includes an explicit decision gate — including a "do not refactor" option.Status
/mnt/raid0nvme0is at 100% (≈2 GB free of 14 TB). A from-source CUDA/C++ rebuild will fail withENOSPCuntil space is freed. This PR is therefore design-only until disk is available.Non-goals
No changes to on-disk format,
name_id_map.json, checkpoint layout, or the numeric values ofcorr_id/is_sparse(only where they are computed). All changes are additive (v2 binding alongside v1) for trivial rollback.See the committed plan for the full task breakdown, verification matrix, risks, and rollback.