implement of leftpadding #2242

pass-lin · 2025-05-02T12:57:00Z

from #2237

mattdangerw

thanks! couple comments

keras_hub/src/layers/preprocessing/start_end_packer.py

pass-lin · 2025-05-09T06:10:13Z

@mattdangerw
hey,plz review my new update
it has passed test from your colab

pass-lin · 2025-05-17T04:26:34Z

=========================== short test summary info ============================
FAILED keras_hub/src/models/gemma/gemma_causal_lm_test.py::GemmaCausalLMTest::test_flash_attention_call - AssertionError: Expected 'dot_product_attention' to have been called.
FAILED keras_hub/src/models/gemma3/gemma3_causal_lm_test.py::Gemma3CausalLMTest::test_text_flash_attention_call - AssertionError: Expected 'dot_product_attention' to have been called.
FAILED keras_hub/src/models/mixtral/mixtral_backbone_test.py::MixtralBackboneTest::test_backbone_basics - AttributeError: 'CachedMixtralAttention' object has no attribute 'dropout'. Did you mean: '_dropout'?
FAILED keras_hub/src/models/mixtral/mixtral_backbone_test.py::MixtralBackboneTest::test_saved_model - AttributeError: 'CachedMixtralAttention' object has no attribute 'dropout'. Did you mean: '_dropout'?
FAILED keras_hub/src/models/mixtral/mixtral_causal_lm_test.py::MixtralCausalLMTest::test_causal_lm_basics - AttributeError: 'CachedMixtralAttention' object has no attribute 'dropout'. Did you mean: '_dropout'?
FAILED keras_hub/src/models/mixtral/mixtral_causal_lm_test.py::MixtralCausalLMTest::test_early_stopping - AttributeError: 'CachedMixtralAttention' object has no attribute 'dropout'. Did you mean: '_dropout'?
FAILED keras_hub/src/models/mixtral/mixtral_causal_lm_test.py::MixtralCausalLMTest::test_generate - AttributeError: 'CachedMixtralAttention' object has no attribute 'dropout'. Did you mean: '_dropout'?
FAILED keras_hub/src/models/mixtral/mixtral_causal_lm_test.py::MixtralCausalLMTest::test_generate_compilation - AttributeError: 'CachedMixtralAttention' object has no attribute 'dropout'. Did you mean: '_dropout'?
FAILED keras_hub/src/models/mixtral/mixtral_causal_lm_test.py::MixtralCausalLMTest::test_saved_model - AttributeError: 'CachedMixtralAttention' object has no attribute 'dropout'. Did you mean: '_dropout'?
FAILED keras_hub/src/models/mixtral/mixtral_causal_lm_test.py::MixtralCausalLMTest::test_score_layer_intercept_fn_exfiltration - AttributeError: 'CachedMixtralAttention' object has no attribute 'dropout'. Did you mean: '_dropout'?
FAILED keras_hub/src/models/mixtral/mixtral_causal_lm_test.py::MixtralCausalLMTest::test_score_logits - AttributeError: 'CachedMixtralAttention' object has no attribute 'dropout'. Did you mean: '_dropout'?
FAILED keras_hub/src/models/mixtral/mixtral_causal_lm_test.py::MixtralCausalLMTest::test_score_loss - AttributeError: 'CachedMixtralAttention' object has no attribute 'dropout'. Did you mean: '_dropout'?
FAILED keras_hub/src/models/qwen_moe/qwen_moe_backbone_test.py::QwenMoeBackboneTest::test_auxiliary_loss - AttributeError: 'QwenMoeAttention' object has no attribute 'logit_soft_cap'
FAILED keras_hub/src/models/qwen_moe/qwen_moe_backbone_test.py::QwenMoeBackboneTest::test_backbone_basics - AttributeError: 'QwenMoeAttention' object has no attribute 'logit_soft_cap'
FAILED keras_hub/src/models/qwen_moe/qwen_moe_backbone_test.py::QwenMoeBackboneTest::test_saved_model - AttributeError: 'QwenMoeAttention' object has no attribute 'logit_soft_cap'
FAILED keras_hub/src/models/qwen_moe/qwen_moe_causal_lm_test.py::QwenMoeCausalLMTest::test_causal_lm_basics - AttributeError: 'QwenMoeAttention' object has no attribute 'logit_soft_cap'
FAILED keras_hub/src/models/qwen_moe/qwen_moe_causal_lm_test.py::QwenMoeCausalLMTest::test_early_stopping - AttributeError: 'QwenMoeAttention' object has no attribute 'logit_soft_cap'
FAILED keras_hub/src/models/qwen_moe/qwen_moe_causal_lm_test.py::QwenMoeCausalLMTest::test_flash_attention_call - AttributeError: 'QwenMoeAttention' object has no attribute 'logit_soft_cap'
FAILED keras_hub/src/models/qwen_moe/qwen_moe_causal_lm_test.py::QwenMoeCausalLMTest::test_generate - AttributeError: 'QwenMoeAttention' object has no attribute 'logit_soft_cap'
FAILED keras_hub/src/models/qwen_moe/qwen_moe_causal_lm_test.py::QwenMoeCausalLMTest::test_generate_compilation - AttributeError: 'QwenMoeAttention' object has no attribute 'logit_soft_cap'
FAILED keras_hub/src/models/qwen_moe/qwen_moe_causal_lm_test.py::QwenMoeCausalLMTest::test_generate_strip_prompt - AttributeError: 'QwenMoeAttention' object has no attribute 'logit_soft_cap'
FAILED keras_hub/src/models/qwen_moe/qwen_moe_causal_lm_test.py::QwenMoeCausalLMTest::test_saved_model - AttributeError: 'QwenMoeAttention' object has no attribute 'logit_soft_cap'
========== 22 failed, 1228 passed, 489 skipped in 5007.81s (1:23:27) =========

@sachinprasadhs It seems that this error has nothing to do with me. But it's worth raising a new issue to fix it.
i can fix this bug at #2257

pass-lin · 2025-05-17T07:30:41Z

@mattdangerw Could you please check if we meet the criteria for merging now?

keras_hub/src/layers/preprocessing/start_end_packer.py

mattdangerw · 2025-05-20T02:16:10Z

keras_hub/src/layers/preprocessing/start_end_packer_test.py

+        expected_output = [[3, 3, 1, 5, 6, 7, 2], [3, 1, 8, 9, 10, 11, 2]]
+        self.assertAllEqual(output, expected_output)
+
+    def test_truncation_side_flips(self):


I'm not sure we need all of these. We might just need test_truncation and test_truncation_without_end_value for these next three tests.

I'm not sure we need all of these. We might just need and for these next three tests.test_truncation``test_truncation_without_end_value

I did both left and right scenarios in all the tests. I think it's still necessary.

Let's at least remove the "side_flips" from the name. I don't think any reader would understand what that means. test_truncation and test_truncation_without_endvalue

keras_hub/src/layers/preprocessing/start_end_packer_test.py

mattdangerw · 2025-05-20T15:26:58Z

keras_hub/src/layers/preprocessing/start_end_packer.py

@@ -139,6 +142,20 @@ def check_special_value_type(value, value_name):

        self.pad_value = pad_value
        self.return_padding_mask = return_padding_mask
+        self.padding_side = padding_side
+
+    def pad(self, x, shape, pad_value):


Pull this into a util in tensor_utils.py or something like that. We will also need it for the multi segment packer.

Pull this into a util in or something like that. We will also need it for the multi segment packer.tensor_utils.py

I don't think this is necessary. Only Bert-like models are using multi-segment packers. For Bert-like models, there is no essential difference between left padding and right padding.

We want it first and foremost for uniformity of the API. But also, this is not just for BERT-like. Gemma3 and PaliGemma use it, for example.

We want it first and foremost for uniformity of the API. But also, this is not just for BERT-like. Gemma3 and PaliGemma use it, for example.

OK, I have fulfilled this requirement. Please check

mattdangerw · 2025-05-20T15:28:52Z

keras_hub/src/layers/preprocessing/start_end_packer_test.py

@@ -147,3 +288,39 @@ def test_get_config(self):
        }

        self.assertEqual(config, {**config, **expected_config_subset})
+
+    def test_return_padding_mask_right_padding(self):


In keeping with the other tests, just leave these in the same test with a # right padding and # left padding comment.

mattdangerw · 2025-05-20T15:31:53Z

keras_hub/src/layers/preprocessing/start_end_packer_test.py

+        expected_output = [[3, 3, 1, 5, 6, 7, 2], [3, 1, 8, 9, 10, 11, 2]]
+        self.assertAllEqual(output, expected_output)
+
+    def test_truncation_side_flips(self):


Let's at least remove the "side_flips" from the name. I don't think any reader would understand what that means. test_truncation and test_truncation_without_endvalue

mattdangerw

Thanks! Just last couple minor comments. This looks good.

mattdangerw · 2025-05-22T21:15:08Z

keras_hub/src/layers/preprocessing/multi_segment_packer.py

@@ -124,6 +125,7 @@ def __init__(
        sep_value=None,
        pad_value=None,
        truncate="round_robin",
+        padding_side="right",


please add a docstring

mattdangerw · 2025-05-22T21:15:25Z

keras_hub/src/layers/preprocessing/multi_segment_packer.py

@@ -163,6 +165,8 @@ def check_special_value_type(value, value_name):

        self.pad_value = pad_value



nit: remove this empty line

pass-lin · 2025-05-24T07:32:00Z

@mattdangerw Can my implementation be merged?

implement of leftpadding

299102c

mattdangerw reviewed May 2, 2025

View reviewed changes

keras_hub/src/layers/preprocessing/start_end_packer.py Outdated Show resolved Hide resolved

keras_hub/src/layers/preprocessing/start_end_packer.py Show resolved Hide resolved

keras_hub/src/layers/preprocessing/start_end_packer.py Show resolved Hide resolved

pass-lin added 2 commits May 3, 2025 15:46

add doc

59627a4

update

5d1b2c0

sachinprasadhs added the kokoro:force-run Runs Tests on GPU label May 16, 2025

kokoro-team removed the kokoro:force-run Runs Tests on GPU label May 16, 2025

mattdangerw reviewed May 20, 2025

View reviewed changes

pass-lin added 3 commits May 20, 2025 20:08

fix

97cada7

format

85bb256

format

6ab5ea9

mattdangerw reviewed May 20, 2025

View reviewed changes

pass-lin added 2 commits May 20, 2025 23:40

update test

f1c55ac

add left padding for segment

8c40279

mattdangerw approved these changes May 22, 2025

View reviewed changes

add doc.

7ceef48

mattdangerw added the kokoro:force-run Runs Tests on GPU label May 28, 2025

kokoro-team removed the kokoro:force-run Runs Tests on GPU label May 28, 2025

mattdangerw merged commit c314f88 into keras-team:master May 28, 2025
10 checks passed

		@@ -163,6 +165,8 @@ def check_special_value_type(value, value_name):

		self.pad_value = pad_value

implement of leftpadding #2242

implement of leftpadding #2242

Uh oh!

Conversation

pass-lin commented May 2, 2025

Uh oh!

mattdangerw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pass-lin commented May 9, 2025

Uh oh!

pass-lin commented May 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pass-lin commented May 17, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattdangerw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pass-lin commented May 24, 2025

Uh oh!

Uh oh!

Uh oh!

pass-lin commented May 17, 2025 •

edited

Loading