Skip to content

Handle invalid dimensions in SinusoidalPositionalEncoding#3615

Merged
zcbenz merged 1 commit into
ml-explore:mainfrom
pchintar:sinusoidal-pe-dims-validation-clean
Jun 5, 2026
Merged

Handle invalid dimensions in SinusoidalPositionalEncoding#3615
zcbenz merged 1 commit into
ml-explore:mainfrom
pchintar:sinusoidal-pe-dims-validation-clean

Conversation

@pchintar

@pchintar pchintar commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Description

nn.SinusoidalPositionalEncoding currently assumes that dims is positive and even, but this requirement is not enforced.

The implementation constructs sinusoidal embeddings from sine/cosine frequency pairs using:

one_zero = 1 - mx.arange(0, dims // 2) / (dims // 2 - 1)

This works for the existing tested case (dims=16), but some edge cases are not handled correctly.

For example, if a user constructs:

nn.SinusoidalPositionalEncoding(3)

the implementation produces only 2 embedding dimensions instead of the requested 3. Similarly, dims=2 results in a division by zero while computing the values used to generate the sinusoidal frequencies, and non-positive values are not rejected explicitly.

This change validates that dims is a positive even integer and handles the dims=2 case separately to avoid the division by zero.

Tests

Added coverage for:

  • dims=2
  • Invalid dimensions: 0, 1, and 3

cc @zcbenz

@zcbenz zcbenz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

@pchintar

pchintar commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks!

thank you @zcbenz

@zcbenz zcbenz merged commit 8f0e8b1 into ml-explore:main Jun 5, 2026
16 checks passed
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