mpt : fix n_ctx

[marella]

Also max_seq_len doesn't seem to be used anywhere. Is that expected?

[ggerganov]

Hm, I'm confused. It looks like max_seq_len corresponds to n_ctx.
Looking at HF repo, it seems that it should be 2048, but here we have it defined as 4096:

https://huggingface.co/mosaicml/mpt-7b/blob/main/config.json#L39

Need to download the model and test it

[marella]

Yeah I also think max_seq_len should be n_ctx.
I also mentioned it to the author who created it in #168 (comment)

[marella]

Should I change max_seq_len to n_ctx? Replit example is using max_seq_len as n_ctx everywhere.

[lukasmoellerch]

The main reason was that we always allocate max seq length which might require too much ram for kv cache on low-ram machines

[ggerganov]

Ah yes, I remember now. Let me think about this some more then.

[ggerganov]

The main reason was that we always allocate max seq length which might require too much ram for kv cache on low-ram machines

Ok, but the way it is implemented now, we allocate with n_ctx = 4096:

ggml/examples/mpt/main.cpp

Line 21 in c2fab8a

int n_ctx = 4096;

This is 2x more than max_seq_len.
What is the reasoning for line 21 ?

[lukasmoellerch]

The main reason was that we always allocate max seq length which might require too much ram for kv cache on low-ram machines

Ok, but the way it is implemented now, we allocate with n_ctx = 4096:

ggml/examples/mpt/main.cpp

Line 21 in c2fab8a

int n_ctx = 4096;

This is 2x more than max_seq_len. What is the reasoning for line 21 ?

It was mainly intended as a fix for running the storywriter model which has a much higher seq length

[marella]

Should we do something like n_ctx = min(max_seq_len, 4096)?

[klosax]

I suggest adding a command line parameter for setting n_ctx. Default value could be 512 as in llama. Maximum value could be max_seq_len.

[ggerganov]

Yes, we can do both of these suggestions

[marella]

Added the min() condition.

I can add the command line parameter later in a separate PR. Can it be added to all models not just MPT?

[lukasmoellerch]

I suggest adding a command line parameter for setting n_ctx. Default value could be 512 as in llama. Maximum value could be max_seq_len.

Yes, I like the suggestion, many new models don’t have an inherent max seq length either, thus setting it to that was a bit arbitrary in the first place.