[WS1] Close embedding and LM-head invariance coverage#243
[WS1] Close embedding and LM-head invariance coverage#243inaniloquentee wants to merge 2 commits into
Conversation
Signed-off-by: inaniloquentee <3051000145@qq.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe PR changes the native LM-head reference to fixed-K row-wise projection, adds explicit-device kernel dispatch, registers embedding and LM-head operator checks, and adds batch-layout, padding, permutation, gradient, and log-probability invariance tests. ChangesBatch-invariant forward path
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rl_engine/kernels/ops/pytorch/linear/lm_head.py`:
- Around line 118-119: Update the empty-input branch in the LM-head
implementation around flat_hidden to return an empty matmul result connected to
hidden and weight instead of hidden.new_empty, preserving autograd for zero-row
inputs. Also add the equivalent zero-row branch to the reference snippets in
tests/test_lm_head.py lines 94-100 and docs/operators/lm_head.md lines 57-61 so
they avoid torch.stack([]); the implementation site is
rl_engine/kernels/ops/pytorch/linear/lm_head.py lines 118-119.
In `@tests/test_issue151_embedding_lm_head_invariance.py`:
- Around line 240-271: Update
test_lm_head_linear_logp_handoff_is_layout_invariant_for_rl_batches to reuse the
existing CUDA/bfloat16 parameterization, covering both CPU/float32 and
CUDA/bfloat16 execution. Before calling NativeLinearLogpOp, alter the hidden
rows selected by ~mask while leaving valid rows unchanged, then retain the
reference and layout-invariance assertions so padded hidden values cannot affect
results.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: daceb832-b7f6-4a13-8c93-19c721132703
📒 Files selected for processing (12)
docs/operators/lm_head.mdrl_engine/executors/deepspeed_trainer.pyrl_engine/kernels/gtest/operator_inputs.pyrl_engine/kernels/gtest/operator_specs.pyrl_engine/kernels/ops/pytorch/linear/lm_head.pyrl_engine/kernels/registry.pyrl_engine/tests/test_dispatch.pytests/test_deepspeed_training_worker.pytests/test_issue151_embedding_lm_head_invariance.pytests/test_lm_head.pytests/test_op_checks.pytests/test_operator_inputs.py
Signed-off-by: inaniloquentee <3051000145@qq.com>
Summary
Relationship to existing PRs
Closes #151
Validation
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests