docs(consolidation): audit outcome — flagged MoE/flash-attn duplicates are not duplicates
Documentation / Build API Documentation (push) Failing after 5s
Documentation / Build User Guide (push) Successful in 6s
CI / Build (macos-latest) (push) Failing after 9s
CI / Build CPU-Only (Explicit) (push) Failing after 11s
CI / Format Check (push) Failing after 12s
CI / Clippy Check (push) Failing after 24s
CI / Build (ubuntu-latest) (push) Failing after 1m7s
CI / Test (macos-latest) (push) Has been skipped
CI / Test (ubuntu-latest) (push) Has been skipped
CI / Python Bindings (maturin) (macos-latest) (push) Has been skipped
CI / Python Bindings (maturin) (ubuntu-latest) (push) Has been skipped
CI / WASM Build + Size Check (push) Has been skipped
CI / Distributed Training Tests (push) Has been skipped
CI / CI Success (push) Failing after 0s
Performance Benchmarks / Run Benchmarks (push) Successful in 1m21s
Documentation / Build API Documentation (push) Failing after 5s
Documentation / Build User Guide (push) Successful in 6s
CI / Build (macos-latest) (push) Failing after 9s
CI / Build CPU-Only (Explicit) (push) Failing after 11s
CI / Format Check (push) Failing after 12s
CI / Clippy Check (push) Failing after 24s
CI / Build (ubuntu-latest) (push) Failing after 1m7s
CI / Test (macos-latest) (push) Has been skipped
CI / Test (ubuntu-latest) (push) Has been skipped
CI / Python Bindings (maturin) (macos-latest) (push) Has been skipped
CI / Python Bindings (maturin) (ubuntu-latest) (push) Has been skipped
CI / WASM Build + Size Check (push) Has been skipped
CI / Distributed Training Tests (push) Has been skipped
CI / CI Success (push) Failing after 0s
Performance Benchmarks / Run Benchmarks (push) Successful in 1m21s
Call-site rewrite pass audited every flagged site; none needed consolidation: metal_moe is an API-consistent backend specialization, modular/router.rs is module-level (not expert-token) routing, glam.rs is disabled dead code with a pre-existing bug (noted for whoever re-enables it), and the three flash-attention "reimplementations" turn out to be planner scaffolding, no-op stubs, and a doc comment — no attention math exists to delegate. jepa_gpu's attention is documented as part of the fused GPU ViT block by design. Verified no regressions: rtx-transformers 961 lib tests pass, jepa_gpu 34/34 with cuda, rtx-training cuda check clean. Co-Authored-By: Claude Fable 5 <[email protected]>
This commit is contained in:
+62
-16
@@ -1,31 +1,77 @@
|
|||||||
# Duplicate-implementation consolidation notes
|
# Duplicate-implementation consolidation notes
|
||||||
|
|
||||||
Recorded during the 2026-07-09 wiring/dead-code sweep. These are known
|
Recorded during the 2026-07-09 wiring/dead-code sweep, updated 2026-07-09
|
||||||
duplications that were **not** consolidated in that pass because doing so
|
during the deferred "rewrite call sites" pass. This pass audited every site
|
||||||
requires rewriting call sites; new code should target the canonical
|
listed below; most turned out not to need (or not to be eligible for) a
|
||||||
implementation listed here.
|
call-site rewrite once actually read in full. Details per item below.
|
||||||
|
|
||||||
## Mixture of Experts
|
## Mixture of Experts
|
||||||
|
|
||||||
Canonical: `rtx-transformers/src/layers/mixture_of_experts/`
|
Canonical: `rtx-transformers/src/layers/mixture_of_experts/` (`MoEConfig`,
|
||||||
|
`Router`, `Expert`, `MixtureOfExperts`).
|
||||||
|
|
||||||
Competing implementations still present (divergent internal APIs):
|
- `rtx-transformers/src/layers/metal_moe.rs` (Metal-specific) — **kept, no
|
||||||
- `rtx-transformers/src/layers/metal_moe.rs` (Metal-specific)
|
change needed.** Its types already mirror the canonical naming 1:1
|
||||||
- `rtx-transformers/src/modular/router.rs` (routing only)
|
(`MetalMoEConfig`/`MetalExpert`/`MetalRouter`/`MetalMoE` vs.
|
||||||
- `rtx-transformers/src/architectures/glam.rs` (architecture-embedded)
|
`MoEConfig`/`Expert`/`Router`/`MixtureOfExperts`), and its config fields
|
||||||
|
(`num_experts`, `top_k`, `hidden_dim`, `expert_hidden_dim`,
|
||||||
|
`capacity_factor`, `dropout`, `aux_loss_weight`, `activation`, `bias`) are
|
||||||
|
identical to `MoEConfig`'s, plus Metal-only extras (`z_loss_weight`,
|
||||||
|
`jitter_noise`, `use_metal`). This is a backend specialization done right;
|
||||||
|
forcing a shared struct would just add an indirection layer for no benefit.
|
||||||
|
- `rtx-transformers/src/modular/router.rs` — **not a duplicate; left as is.**
|
||||||
|
This is dynamic *module* selection (`ModuleRouter`, `TaskConditionedRouter`,
|
||||||
|
`AttentionRouter`, `RLRouter`) over `Vec<Box<dyn Module>>` for the modular
|
||||||
|
neural network system (`modular/modular_network.rs`), not token-to-FFN-expert
|
||||||
|
gating. It's wired into `ModuleRouter` trait impls in `modular_network.rs`
|
||||||
|
and has its own test suite (`modular_networks_tests.rs`). A genuinely
|
||||||
|
different abstraction — MoE's `Router` routes tokens to FFN experts by
|
||||||
|
learned gating logits; this routes whole inputs to heterogeneous modules
|
||||||
|
(including RL-policy-based and task-conditioned selection). Not merging.
|
||||||
|
- `rtx-transformers/src/architectures/glam.rs` — **out of scope, dead code.**
|
||||||
|
`mod glam` is commented out in `architectures/mod.rs` ("Temporarily
|
||||||
|
disabled"), so this file is not part of the build at all. It also does not
|
||||||
|
currently compile on its own: `GLaMBlock::forward` references an undefined
|
||||||
|
`routing_info` binding (line ~229) — a pre-existing bug unrelated to
|
||||||
|
consolidation. Migrating its expert layer to
|
||||||
|
`layers::mixture_of_experts::{MixtureOfExperts, MoEConfig, Router}` is the
|
||||||
|
right call *when* someone re-enables this module, but doing that migration
|
||||||
|
on dead, non-compiling code with zero call sites and no test coverage today
|
||||||
|
would be unverifiable busywork. Left as a note for whoever re-enables GLaM.
|
||||||
|
|
||||||
Deleted in the sweep (were orphaned, never declared by `mod`):
|
Deleted in the previous sweep (were orphaned, never declared by `mod`):
|
||||||
`layers/moe_layer.rs`, `layers/moe_integration.rs`.
|
`layers/moe_layer.rs`, `layers/moe_integration.rs`.
|
||||||
|
|
||||||
## Flash Attention
|
## Flash Attention
|
||||||
|
|
||||||
Canonical: the `rtx-flash-attention` crate (v2+v3 kernels, CUDA + Metal).
|
Canonical: the `rtx-flash-attention` crate (v2+v3 kernels, CUDA + Metal,
|
||||||
|
public API: `flash_attention_forward`/`flash_attention_backward` and the
|
||||||
|
`FlashAttention`/`FlashAttentionBackend` types over `rtx_tensor::Tensor`).
|
||||||
|
|
||||||
Reimplementations inside rtx-transformers that should eventually delegate:
|
Re-audited the three non-JEPA sites previously flagged as reimplementations.
|
||||||
- `src/tensor_core_kernels.rs`
|
None of them actually contain duplicate attention math to delegate:
|
||||||
- `src/revolutionary/orchestrator_core.rs`
|
- `src/tensor_core_kernels.rs` — only config/strategy scaffolding
|
||||||
- `src/ssl/jepa_gpu.rs`
|
(`AttentionConfig`, `SoftmaxOptimizationStrategy`,
|
||||||
- `src/training/training_loop.rs`
|
`AttentionComputationOptimizer`, etc.). There is no `compute`/`forward`
|
||||||
|
method that performs Q@K^T/softmax/@V anywhere in the file — it's a set of
|
||||||
|
enums and constructors for a planner that was never wired to real kernels.
|
||||||
|
Nothing to replace.
|
||||||
|
- `src/revolutionary/orchestrator_core.rs` — `execute_flash_attention` (and
|
||||||
|
its siblings `execute_classical`, `execute_hybrid`) are all identical
|
||||||
|
no-op stubs (`Ok(input.clone())`). There's no q/k/v split or attention
|
||||||
|
computation to delegate; this orchestrator picks a "compute modality" by
|
||||||
|
score but was never wired to any real backend for any modality, not just
|
||||||
|
this one. Wiring it up is a separate feature task, not a consolidation.
|
||||||
|
- `src/training/training_loop.rs` — the only mention of `FlashAttention` is a
|
||||||
|
doc comment listing it as an example LLM op; no implementation present.
|
||||||
|
|
||||||
|
`src/ssl/jepa_gpu.rs` is intentionally **not** in this list: its attention is
|
||||||
|
part of the fused GPU-resident ViT block written for the JEPA platform, not
|
||||||
|
a standalone duplicate of `rtx-flash-attention`'s API — the fusion is the
|
||||||
|
point (avoids materializing intermediate tensors across the ViT block same
|
||||||
|
as flash attention's inner loop does, but tied to ViT-specific tensor
|
||||||
|
layouts). Revisit only if `rtx-flash-attention` grows a fused-block API that
|
||||||
|
covers this shape.
|
||||||
|
|
||||||
## Speculative decoding
|
## Speculative decoding
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user