If Alibi is on, we should turn learned_pos_emb to False

[bcui19]

If we use alibi we need to turn the learned_pos_emb to False. Otherwise we still go down the learned_pos_emb codepath.

[vchiley]

Similarly the argument could be made that if alibi == False set learned_pos_emb = True BUT I figured alibi = False and learned_pos_emb = False is NoPE so I left the option open.
Similarly, I wanted to give ppl the ability to use both alibi and learned_pos_emb

maybe add a warning or something???
maybe just ignore me... 😅

[bcui19]

Hm... Okay, so one thing is if we train models w/ ALiBi (without learned_pos_embed) and load that model with learned_pos_embed=True it could really affect model performance (since now you have this random position embedding...)

I also am not 100% sure what's the right thing. The "safest" thing might be to add a warning that we're changing it? Since if a user specifies ALiBi they usually mean the case where you don't add in the positional embedding.

[vchiley]

if we load a trained model, should the model config be part of the ckpt?

In general, the change you made + a warning sounds good.

[samhavens]

would a good middle ground being changing the default value so that learned_pos_emb: bool = False,? Alternatively, we can default learned_pos_emb to None, and then later change the None to learned_pos_emb = not alibi. Users could still explicitly set both to be on, but we shouldn't run in to accidentally having both?

[bcui19]

@vchiley It might be good to independently save it from composer, since also the composer checkpoint is unwieldy.

@samhavens I agree, so I did sth like this:

        if self.learned_pos_emb is None:
            self.learned_pos_emb = not self.attn_config.get('alibi', False)
            warnings.warn(f'`learned_pos_emb` has not been set, setting it to {self.learned_pos_emb}')

And it leads to this super weird warning... ):

Let me know if I'm doing something obviously wrong (I think there's something weird where in transformers where the class gets initialized twice...:
https://github.com/huggingface/transformers/blob/main/src/transformers/configuration_utils.py#L816

And we also can't set this up in _validate_config since we need it if we were to create a model using from_pretrained.

So I think unfortunately, I think I'm just going to add an additional warning, unless someone else has a way around this.

[vchiley]

@bcui19 it sets it to false, then sets it to true?

[bcui19]

I think... When you create MPTConfig the way we do it now, HF does something weird and it ends up creating two MPTConfig's...

[samhavens]

If HF is going to make us choose between extremely cryptic warnings like that or just "set pos_emb to false if alibi is true" then I guess we should revert to what you had before and just let the user know they specified alibi so we are setting pos emb to false.

Unless, does setting the default differently solve this? learned_pos_emb: bool = False ?

[bcui19]

Running w/ the current set of changes:

Going to merge. This is partly me assuming that very few people would want to mix positional embeddings w/ Alibi