CLIP Tokenizer unable to take text of unequal length (or token length) about mlx-examples HOT 7 OPEN

the tokenizer is doing this:

return mx.array([self.tokenize(t, prepend_bos, append_eos) for t in text])

if text is a List[str], and mx.array doesn't like non uniform list of numbers.

I can "hack" this to work by padding this here with EOT. I went over to HF and they seemed to also "hack" this??

pad_token="<|endoftext|>", # hack to enable padding

so just to be consistent with it.

I have this local changes done. I am not sure if this repos is for edu purpose on how to use MLX and not being completely robust and faithful port of huggingface, otherwise, I can do a PR if this fix helps.

from mlx-examples.

I think it would be fine to pad in the tokenizer. Usually we try to keep the examples simple so they are useful for educational purposes / starting points for building from. In this case I don't think it will add much complexity to pad in the tokenizer.

But I guess the clip_loss would be a bit off if you don't consider the padding?

from mlx-examples.

Understood. On the way, i took a quick look at the tokenizer and found probably wasn’t the goal to replicate the HF completely. I followed closely what HF and fixed it locally, and it worked fine. HF tokenizer use end-of-Text to pad, so it is at least as correct as HF seemed to have it.

I wasn’t into training yet so not sure if the clip loss wants to be taken into account the padding method.

from mlx-examples.

I wasn’t into training yet so not sure if the clip loss wants to be taken into account the padding method.

Right but we do evaluate the loss in the current example even when not training. I'm not entirely sure why. Maybe we should remove it o/w it would be incorrect in the padded case.

from mlx-examples.

@kechan where do you add the pad_token="<|endoftext|>", ? Thanks

from mlx-examples.