huggingface / huggingface/blog

Is the implementation of SinusoidalPositionEmbeddings in diffusion tutorial correct ?

Open
#2,183 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Jupyter Notebook
Stars
3.5k
Forks
1k
Avg merge
1d 20h
Merged PRs (30d)
19

Description

```
class SinusoidalPositionEmbeddings(nn.Module):
def __init__(self, dim):
super().__init__()
self.dim = dim

def forward(self, time):
device = time.device
half_dim = self.dim // 2
embeddings = math.log(10000) / (half_dim - 1)
embeddings = torch.exp(torch.arange(half_dim, device=device) * -embeddings)
embeddings = time[:, None] * embeddings[None, :]
embeddings = torch.cat((embeddings.sin(), embeddings.cos()), dim=-1)
return embeddings
```
In the tutorial : https://github.com/huggingface/blog/blob/main/annotated-diffusion.md

for a given time-step, will this implementation not place all the sines first, and all the cosines later. (due to the line `embeddings = torch.cat((embeddings.sin(), embeddings.cos()), dim=-1)`)

should it not be alternating sines and cosines like `sin , cos, sin, cos ... and so on` ?

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with the SinusoidalPositionEmbeddings implementation in annotated-diffusion.md and inspect how its output is consumed in the diffusion tutorial. Verify whether the sine/cosine ordering matches the intended positional embedding convention, then document the conclusion or update the tutorial if the implementation is incorrect.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, pytorch
Domain
documentation, machine-learning
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.