Skip to content

Conversation

@LoserCheems
Copy link
Collaborator

Implements two rotary embedding variants for transformer attention:

  • Interleaved format with real/imaginary components alternating
  • Contiguous format with separate real/imaginary halves

Both functions support boundary checking, out-of-bounds clearing, and convert between data types for precise floating-point calculations.

Implements two rotary embedding variants for transformer attention:
- Interleaved format with real/imaginary components alternating
- Contiguous format with separate real/imaginary halves

Both functions support boundary checking, out-of-bounds clearing,
and convert between data types for precise floating-point calculations.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces two implementations for rotary positional encoding to support transformer attention.

  • Implements an interleaved variant, where real/imaginary components alternate.
  • Implements a contiguous variant, with separate real/imaginary halves and additional pointer arithmetic for signal extraction.
Comments suppressed due to low confidence (2)

csrc/src/rotary.h:65

  • Please replace the informal 'Idk' comment with a clear explanation detailing why the extra copy is required for convert_type to work correctly.
                        // Idk but I need to copy for the convert_type to work

csrc/src/rotary.h:134

  • Please update the comment to explicitly clarify the necessity of copying the tensor after conversion, to improve maintainability and clarity of intent.
                        // Idk but I need to copy for the convert_type to work

cute::copy(S(_, m, k), rS(_, m, k));
if (get<1>(identity_MN(0, 0, k)) < rotary_dim) {
const bool is_left = get<1>(identity_MN(0, 0, k)) < rotary_dim / 2;
Tensor gS_other = make_tensor(S(_, m, k).data() + (is_left ? rotary_dim / 2 : -rotary_dim / 2), S(_, m, k).layout());
Copy link

Copilot AI Jun 23, 2025

Choose a reason for hiding this comment

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

The use of negative pointer offset here may raise safety concerns; please add a comment or safeguard to clarify that accessing memory with a negative offset is valid and correctly bounded.

Copilot uses AI. Check for mistakes.
@LoserCheems LoserCheems merged commit 73caf73 into main Jun 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants