Skip to content

Fix fused gate rule#580

Open
AndyLi429 wants to merge 2 commits into
sgl-project:mainfrom
AndyLi429:fix_fused_gate_rule
Open

Fix fused gate rule#580
AndyLi429 wants to merge 2 commits into
sgl-project:mainfrom
AndyLi429:fix_fused_gate_rule

Conversation

@AndyLi429

Copy link
Copy Markdown
Contributor

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the offset calculation in the fused_sigmoid_gating_delta_rule_update_npu_kernel to use a dv-major layout and adds a new test suite to verify decode parity. The reviewer notes that the test asserting on the exact string representation of the source code is fragile and redundant, suggesting its removal to improve maintainability.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +13 to +18
def test_decode_h0_state_uses_dv_major_layout(self):
source = FUSED_SIGMOID_GATING_RECURRENT.read_text(encoding="utf-8")
compact = "".join(source.split())

self.assertNotIn("+o_k[:,None]*V+o_v[None,:]", compact)
self.assertEqual(compact.count("+o_v[None,:]*K+o_k[:,None]"), 2)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Asserting on the exact string representation of the source code is a fragile testing practice. If any variable names (like o_v, K, o_k) are renamed or if the slicing syntax is slightly adjusted in the future, this test will fail even if the kernel's logic remains perfectly correct. Since test_verify_matches_serial_decode_with_dk128_dv64 already functionally verifies the correctness of the layout and offset calculation, this string-matching test is redundant and can be safely removed to improve maintainability.

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.

1 participant