Discussion: Update dataloader to skip rows that dont require training

[felipemello1]

#2341

When a) train_on_input=False and b) message is too long that output is truncated, there may be a batch without trainable tokens, raising an error on the loss because of division by zero.

Beyond raising an inconvenient bug, this is a waste of compute, and fixing the loss seems to be fixing a symptom, instead of the root cause.

In the dataloader, should we skip rows that dont have trainable embeddings?

[RdoubleA]

It's difficult to know if this is the case fro the DataLoader perspective. You would have to catch this and raise/skip in the recipe or add additional preprocessing to your dataset to prevent this. We don't have the ability to skip an item within the SFTDataset's __getitem__.

[felipemello1]

I see. It could be added to the recipe:

for batch in dataloader():
	if not check_batch_requires_grad(batch):
		continue

I am sure @ebsmothers will love the idea!

[EugenHotaj]

Maybe slightly orthogonal but it would also be great to expose a way to do left truncation instead of right truncation. I think this is even more important once the "mask all previous turns" strategy is introduced.

(I'm actually a bit surprised to see that torchtune supports left truncation only since I thought right truncation was the standard practice for llama models).

[pocca2048]

You would have to catch this and raise/skip in the recipe

I agree. Actually, I implemented skipping logic in the recipe w/o fixing the loss.