Additional positional embeddings

[bonham79]

@Adamits I'll implement but want to check with you if it's worthwhile since my domain is speech:

What are your thoughts on adding in new positional embeddings to Transformer models (particularly RoPE)? iirc we're using the standard cosine ones but they're a bit old fashioned nowadays. Know if there's arguments for or against?

[kylebgorman]

+1 I was wondering about this too, do our characteristic problems have enough data for learned positional embeddings?

[Adamits]

In general, I think the method we use is not considered the best one, but no idea to what extent it matters in our small-vocab character domains. Its actually a sort of interesting problem since I would think in problems with monotonic-ish alignments position representations would be very important.

I definitely think we should implement alternatives. Funny enough, a friend who works in this space and has used Yoyodyne for baselines texted me yesterday asking if we had RoPe style embeddings so it sounds like a great feature.

[bonham79]

Kk, i'll implement them and we can see how they work off a fork. From some posts it looks like ~10 loc so not the craziest implementation. If we're seeing overfitting we can try to apply some context window limiting or the like.

Main things I'm thinking

• RoPE
• Absolute embeddings
• No embedding (NoPE) (some people have been arguing PEs aren't really that important, so seems like worthwhile option)

[kylebgorman]

Suggested interface: --transformer_positional_embedding {nope,rope,sinusoidal} (etc.). I would be very happy to just have RoPE as a second option.

[bonham79]

For sinusoidal, do we want them to auto expand or enforce a max_seq_length hp? (Asking because this ruined my life last weekend...)

[kylebgorman]

Correct me if I'm wrong, but we already have a max sequence length built into our transformers anyways, I think. If you can lift this restriction, sure, but otherwise I'd just obey it.

[bonham79]

Yeah, we need it at minimum to stop long tail cases on inference. For our models it may not really be necessary (may be a fun experiment to run). For cosine encodings the only reason to include would be consistency with other embeddings ui and efficiency - avoids needing to compute the cosine values per batch. Thinking I'll just enforce the constraint until we encounter a case that says otherwise.

[Adamits]

Yeah pretty sure the current sin implementation relies on max_seq_length.