Skip to content

Enforce half-split layout for cos/sin in RoPE challenge test data#271

Open
baqizhaobenshan wants to merge 1 commit into
AlphaGPU:mainfrom
baqizhaobenshan:fix/rope-half-split-test-data
Open

Enforce half-split layout for cos/sin in RoPE challenge test data#271
baqizhaobenshan wants to merge 1 commit into
AlphaGPU:mainfrom
baqizhaobenshan:fix/rope-half-split-test-data

Conversation

@baqizhaobenshan

Copy link
Copy Markdown

Summary

Fixes the RoPE challenge test data so cos and sin follow the half-split layout described by the problem.
For RoPE with rotate_half, paired dimensions j and j + D/2 should use the same cosine and sine values. Previously, several generated test cases used fully independent random values for all D dimensions, so cos[:, j] != cos[:, j + D/2] and sin[:, j] != sin[:, j + D/2] in general.
This PR keeps the existing random data generation style unchanged and only enforces:

value[:, D // 2 :] = value[:, : D // 2]

Changes

  • Added a small helper to enforce half-split layout for generated cos and sin tables.
  • Updated generated RoPE test inputs so paired dimensions share the same cos / sin values.
  • Updated the fixed mixed_values test case to satisfy the same layout.
  • Documented the half-split cos / sin requirement in the challenge description.

Motivation

The current random cos / sin test data prevents implementations from using the standard half-split RoPE assumption that paired dimensions share the same rotation values. This makes the test data inconsistent with the expected RoPE layout and forces extra cos / sin loads that should not be necessary for this formulation.

Testing

  • python3 -m py_compile challenges/medium/61_rope_embedding/challenge.py
  • git diff --check -- challenges/medium/61_rope_embedding/challenge.py challenges/medium/61_rope_embedding/challenge.html

@baqizhaobenshan

Copy link
Copy Markdown
Author

Hi maintainers, gentle ping on this PR.

I verified the change locally with:

  • black --check challenges/medium/61_rope_embedding/challenge.py
  • isort --check-only challenges/medium/61_rope_embedding/challenge.py
  • flake8 challenges/medium/61_rope_embedding/challenge.py
  • python3 -m py_compile challenges/medium/61_rope_embedding/challenge.py
  • git diff --check -- challenges/medium/61_rope_embedding/challenge.py challenges/medium/61_rope_embedding/challenge.html

All checks passed locally. The Checks tab currently shows “completed with no jobs”, so I am not sure whether CI was skipped because of path filters or fork permissions.

The intended invariant here is that RoPE uses rotate_half / half-split layout, so dimensions j and j + D/2 form a pair and should share the same cos/sin values. The current random test generation samples all D dimensions independently, which breaks this invariant. This PR only fixes the test data generation and documents the invariant; it does not change the solve signature or the expected output formula.

@kunal-mansukhani Sorry for the ping — when you have time, could you take a look at whether this matches the intended RoPE layout for this challenge?

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