Repository navigation
Add TDT loss kernel #46048
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add TDT loss kernel #46048
Changes from all commits
fa7d6e0
f2b4938
fa36657
05e2e34
bb5ff33
9ec79b0
33f128e
760b4b6
b33002f
48b39dd
e9f23ab
e2b97aa
6b9fc73
6c879bc
149e17f
36bfa63
2df0ccc
388c6d3
08b2b55
0c4e05a
1ddd804
f512670
07d8e35
fab050a
c438565
86d980c
895c4a0
ab21380
d0141d5
f7529d4
77b95d7
94eae66
f7d4067
f75c17b
5a49b65
881233f
b41a8ee
897753a
6c914db
756cee1
f30c536
fa95fc8
5df7f28
cd706d4
a47ed8a
13b68ce
72c1ad0
8e23b3d
1cc39fd
531f297
2c0f23a
cef6639
fd3cf9b
e63a5bf
f9d1a4f
43ee7cd
c2a0f78
1fd7ed7
7cc9d2e
e753eab
ed3fa4d
ab66b23
a5ba0c6
48279a6
1d7680d
59ddced
31490d1
d8eb1b6
1f1b912
9ab08d1
fd9f8b1
136f676
833d289
a1c62a1
8683570
1d4b0f4
a418eca
5d0c631
5c603c1
09ba99c
e743b2d
8d09cb6
5176486
1cbb74f
12bbc03
475304e
28b0a40
d6e739c
3f02948
5bd9cac
0a4ccb2
513ea4b
8c0d977
3321659
3d23b40
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Imo no need to test it on this side as the kernel should test it for itself no? Meaning kernels community ensures that the loss is correct 🤔
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense, the kernel tests in kernels-community already check it against a PyTorch reference, a naive loop version and float64. Done in 3f02948: I removed the loss-correctness tests here and restored the integration test, keeping only Also tested against your staged build ( |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,16 @@ | |
| from pathlib import Path | ||
|
|
||
| from transformers import is_datasets_available, is_torch_available | ||
| from transformers.testing_utils import cleanup, require_torch, require_torchaudio, slow, torch_device | ||
| from transformers.testing_utils import ( | ||
| cleanup, | ||
| preserve_module_forwards, | ||
| require_kernels, | ||
| require_torch, | ||
| require_torch_gpu, | ||
| require_torchaudio, | ||
| slow, | ||
| torch_device, | ||
| ) | ||
|
|
||
| from ...test_configuration_common import ConfigTester | ||
| from ...test_modeling_common import ModelTesterMixin, floats_tensor, ids_tensor, random_attention_mask | ||
|
|
@@ -608,6 +617,41 @@ def create_and_check_model(self, config, inputs_dict): | |
| (self.batch_size, self.output_seq_length, self.encoder_model_tester.hidden_size), | ||
| ) | ||
|
|
||
| def prepare_config_and_inputs_for_loss(self): | ||
| config, input_features, attention_mask = self.prepare_config_and_inputs() | ||
| labels = ids_tensor([self.batch_size, 6], self.blank_token_id) # no blank in the labels | ||
| labels[labels == self.pad_token_id] = self.pad_token_id + 1 # nor padding | ||
| blank = torch.full((self.batch_size, 1), self.blank_token_id, device=labels.device) | ||
| inputs_dict = { | ||
| "input_features": input_features, | ||
| "attention_mask": attention_mask, | ||
| "decoder_input_ids": torch.cat([blank, labels], dim=1), | ||
| "labels": labels, | ||
| } | ||
| return config, inputs_dict | ||
|
|
||
| def create_and_check_use_kernels(self, config, inputs_dict): | ||
| """`use_kernels=True` swaps `tdt_loss` for the `kernels-community/tdt-loss` kernel, with the same loss.""" | ||
| model = ParakeetForTDT(config).to(torch_device).eval() | ||
| with torch.no_grad(): | ||
| expected = model(**inputs_dict).loss | ||
|
|
||
| with preserve_module_forwards(model): | ||
| model.set_use_kernels(True) | ||
| self.parent.assertIn("forward", vars(tdt_loss), "`tdt_loss` was not kernelized") | ||
| with torch.no_grad(): | ||
| loss = model(**inputs_dict).loss | ||
| torch.testing.assert_close(loss, expected, rtol=1e-4, atol=1e-4) | ||
| self.parent.assertNotIn("forward", vars(tdt_loss), "`tdt_loss` was not restored") | ||
|
|
||
| # Training mode also uses the kernel, which has a backward | ||
| with preserve_module_forwards(model): | ||
| model.train() | ||
| model.set_use_kernels(True) | ||
| self.parent.assertIn("forward", vars(tdt_loss), "`tdt_loss` was not kernelized in training mode") | ||
| model(**inputs_dict).loss.backward() | ||
| self.parent.assertTrue(any(p.grad is not None for p in model.parameters()), "No gradients after backward") | ||
|
|
||
| def prepare_config_and_inputs_for_common(self): | ||
| config, input_features, attention_mask = self.prepare_config_and_inputs() | ||
| decoder_input_ids = ids_tensor([self.batch_size, 1], self.vocab_size) | ||
|
|
@@ -650,6 +694,12 @@ def test_model(self): | |
| config_and_inputs = self.model_tester.prepare_config_and_inputs_for_common() | ||
| self.model_tester.create_and_check_model(*config_and_inputs) | ||
|
|
||
| @require_torch_gpu | ||
| @require_kernels | ||
| def test_use_kernels(self): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets make the split as in
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in 513ea4b: the logic is now in |
||
| config_and_inputs = self.model_tester.prepare_config_and_inputs_for_loss() | ||
| self.model_tester.create_and_check_use_kernels(*config_and_inputs) | ||
|
|
||
| @unittest.skip(reason="ParakeetForTDT does not use inputs_embeds") | ||
| def test_model_get_set_embeddings(self): | ||
| pass | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might have to drop the kernel for when we make use kernels true as default. best to make it toch compile friendly from the get go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, done. The kernel now supports
torch.compile(huggingface/kernels-community#882, 02647af): fake implementations for its ops,can_torch_compile = Trueon the layer, and tests withtorch.compile(fullgraph=True)(fp32/bf16, forward and backward match eager) andtorch.library.opcheck. Here, the mapping is nowMode.TRAINING | Mode.TORCH_COMPILE(5bd9cac), like the liger loss.Checked on GPU with a build of the new kernel: a
ParakeetForTDTkernelized withMode.TRAINING | Mode.TORCH_COMPILEand a compiledtdt_lossmatch eager, and the Parakeet tests pass.