From e1884639e3f6487a287bfee9588a259bc02e064c Mon Sep 17 00:00:00 2001 From: Jay Gala <57001778+jaygala223@users.noreply.github.com> Date: Sat, 11 Mar 2023 04:04:05 +0530 Subject: [PATCH 1/5] added conda warning in CONTRIBUTING.md (#805) --- CONTRIBUTING.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6b51e960a0..d5af724c00 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -97,6 +97,10 @@ recommend using `conda` to install tensorflow dependencies (such as CUDA), and `pip` to install python packages from PyPI. The exact method will depend on your OS. +**Note**: Please be careful not to use the `tensorflow` pre-packaged with conda, +which is incompatible with `tensorflow-text` on PyPi, and follow the +instructions below. + ### Linux (recommended) To setup a complete environment with TensorFlow, a local install of keras-nlp, From d838bbd9c9bb6b27519dec6c7530e4e9d04102fd Mon Sep 17 00:00:00 2001 From: Jay Gala <57001778+jaygala223@users.noreply.github.com> Date: Sat, 11 Mar 2023 04:04:13 +0530 Subject: [PATCH 2/5] fixed minor grammatical error in ROADMAP.md (#800) --- ROADMAP.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ROADMAP.md b/ROADMAP.md index 4d92b2fe47..ca7c112f7f 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -148,7 +148,7 @@ are trying to explain to the reader. This will continue to be a key investment area for the library. If you have an idea for a guide or example, please open an issue to discuss. -By the end of 2022, most new NLP examples on keras.io should be use +By the end of 2022, most new NLP examples on keras.io should use KerasNLP library. ## Citation bar From b2df61a66c3107792b092940a2a98520e0e1079a Mon Sep 17 00:00:00 2001 From: Anshuman Mishra <51750587+shivance@users.noreply.github.com> Date: Sat, 11 Mar 2023 05:20:54 +0530 Subject: [PATCH 3/5] Remove API export decorator from base classes (#824) * removing exports from base classes * addressing comments --- keras_nlp/models/backbone.py | 3 +-- keras_nlp/models/preprocessor.py | 3 +-- keras_nlp/models/task.py | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/keras_nlp/models/backbone.py b/keras_nlp/models/backbone.py index 8639c5ca41..78e7d46c91 100644 --- a/keras_nlp/models/backbone.py +++ b/keras_nlp/models/backbone.py @@ -18,12 +18,11 @@ from tensorflow import keras -from keras_nlp.api_export import keras_nlp_export from keras_nlp.utils.python_utils import classproperty from keras_nlp.utils.python_utils import format_docstring -@keras_nlp_export("keras_nlp.models.Backbone") +@keras.utils.register_keras_serializable(package="keras_nlp") class Backbone(keras.Model): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) diff --git a/keras_nlp/models/preprocessor.py b/keras_nlp/models/preprocessor.py index ad8702de9f..546cc7639b 100644 --- a/keras_nlp/models/preprocessor.py +++ b/keras_nlp/models/preprocessor.py @@ -14,12 +14,11 @@ from tensorflow import keras -from keras_nlp.api_export import keras_nlp_export from keras_nlp.utils.python_utils import classproperty from keras_nlp.utils.python_utils import format_docstring -@keras_nlp_export("keras_nlp.models.Preprocessor") +@keras.utils.register_keras_serializable(package="keras_nlp") class Preprocessor(keras.layers.Layer): """Base class for model preprocessors.""" diff --git a/keras_nlp/models/task.py b/keras_nlp/models/task.py index 0d3f1d5427..8ea1e2439a 100644 --- a/keras_nlp/models/task.py +++ b/keras_nlp/models/task.py @@ -17,13 +17,12 @@ from tensorflow import keras -from keras_nlp.api_export import keras_nlp_export from keras_nlp.utils.pipeline_model import PipelineModel from keras_nlp.utils.python_utils import classproperty from keras_nlp.utils.python_utils import format_docstring -@keras_nlp_export("keras_nlp.models.Task") +@keras.utils.register_keras_serializable(package="keras_nlp") class Task(PipelineModel): """Base class for Task models.""" From 89a9d4683a24493a2b57a5185dc3c4fdf43b8810 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chollet?= Date: Fri, 10 Mar 2023 17:19:12 -0800 Subject: [PATCH 4/5] Move integration tests out of repo sources. (#826) * Move integration tests out of repo sources. * Add integration test workflow to nightly actions. --- .github/workflows/actions.yml | 3 +++ .github/workflows/nightly.yml | 3 +++ .../tests/integration_tests => integration_tests}/__init__.py | 0 .../basic_usage_test.py | 0 .../integration_tests => integration_tests}/import_test.py | 0 5 files changed, 6 insertions(+) rename {keras_nlp/tests/integration_tests => integration_tests}/__init__.py (100%) rename {keras_nlp/tests/integration_tests => integration_tests}/basic_usage_test.py (100%) rename {keras_nlp/tests/integration_tests => integration_tests}/import_test.py (100%) diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml index 343d8203fc..14a5ab7be5 100644 --- a/.github/workflows/actions.yml +++ b/.github/workflows/actions.yml @@ -34,6 +34,9 @@ jobs: - name: Test with pytest run: | pytest --cov=keras_nlp --cov-report xml:coverage.xml + - name: Run integration tests + run: | + python pip_build.py --install && cd integration_tests && pytest . format: name: Check the code format runs-on: ubuntu-latest diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index fd62f43f01..1c998b0514 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -39,3 +39,6 @@ jobs: - name: Test with pytest run: | pytest --cov=keras_nlp --cov-report xml:coverage.xml + - name: Run integration tests + run: | + python pip_build.py --install && cd integration_tests && pytest . diff --git a/keras_nlp/tests/integration_tests/__init__.py b/integration_tests/__init__.py similarity index 100% rename from keras_nlp/tests/integration_tests/__init__.py rename to integration_tests/__init__.py diff --git a/keras_nlp/tests/integration_tests/basic_usage_test.py b/integration_tests/basic_usage_test.py similarity index 100% rename from keras_nlp/tests/integration_tests/basic_usage_test.py rename to integration_tests/basic_usage_test.py diff --git a/keras_nlp/tests/integration_tests/import_test.py b/integration_tests/import_test.py similarity index 100% rename from keras_nlp/tests/integration_tests/import_test.py rename to integration_tests/import_test.py From 4c94a0d68872a72ed3e27d3f93a3d7eec3ddc0ba Mon Sep 17 00:00:00 2001 From: Alexandre Bodinier <70907397+abodinier@users.noreply.github.com> Date: Sat, 11 Mar 2023 03:39:29 +0100 Subject: [PATCH 5/5] Function merge_padding_and_attention_mask does not return an output with the desired shape when both padding and attention masks are given (#790) * Remove duplicated tf.newaxis * Create shapes check for padding and attention masks * Update tests * Fix lint issue * More lint fixes * Update error messages --------- Co-authored-by: Matt Watson --- keras_nlp/layers/transformer_layer_utils.py | 26 +++++++++++++++--- .../layers/transformer_layer_utils_test.py | 27 ++++++++++++++++++- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/keras_nlp/layers/transformer_layer_utils.py b/keras_nlp/layers/transformer_layer_utils.py index 2ce24d8a5b..9bf88aec9a 100644 --- a/keras_nlp/layers/transformer_layer_utils.py +++ b/keras_nlp/layers/transformer_layer_utils.py @@ -18,6 +18,26 @@ from absl import logging +def _check_masks_shapes(inputs, padding_mask, attention_mask): + mask = padding_mask + if hasattr(inputs, "_keras_mask") and mask is None: + mask = inputs._keras_mask + if mask is not None: + if mask._rank() != 2: + raise ValueError( + "`padding_mask` should have shape " + "(batch_size, target_length). " + f"Received shape `{mask.shape}`." + ) + if attention_mask is not None: + if attention_mask._rank() != 3: + raise ValueError( + "`attention_mask` should have shape " + "(batch_size, target_length, source_length). " + f"Received shape `{mask.shape}`." + ) + + def compute_causal_mask(batch_size, input_length, output_length, cache_index=0): """Compute a causal attention mask for a transformer decoder. @@ -60,6 +80,7 @@ def merge_padding_and_attention_mask( A merged 2D mask or None. If only `padding_mask` is provided, the returned mask is padding_mask with one additional axis. """ + _check_masks_shapes(inputs, padding_mask, attention_mask) mask = padding_mask if hasattr(inputs, "_keras_mask"): if mask is None: @@ -80,8 +101,5 @@ def merge_padding_and_attention_mask( if mask is None: return attention_mask else: - return tf.minimum( - mask[:, tf.newaxis, :], - attention_mask, - ) + return tf.minimum(mask, attention_mask) return mask diff --git a/keras_nlp/layers/transformer_layer_utils_test.py b/keras_nlp/layers/transformer_layer_utils_test.py index 82ee1d93f2..462aaab176 100644 --- a/keras_nlp/layers/transformer_layer_utils_test.py +++ b/keras_nlp/layers/transformer_layer_utils_test.py @@ -24,7 +24,9 @@ def test_compute_causal_mask(self): def test_merge_padding_and_attention_mask(self): padding_mask = tf.convert_to_tensor([[1, 1, 0]]) - attention_mask = tf.convert_to_tensor([[0, 0, 1], [0, 1, 0], [1, 0, 0]]) + attention_mask = tf.convert_to_tensor( + [[[0, 0, 1], [0, 1, 0], [1, 0, 0]]] + ) inputs = tf.random.uniform(shape=[1, 3, 2]) merged_mask = utils.merge_padding_and_attention_mask( inputs, @@ -34,3 +36,26 @@ def test_merge_padding_and_attention_mask(self): self.assertTrue( (merged_mask.numpy() == [[0, 0, 0], [0, 1, 0], [1, 0, 0]]).all() ) + + def test_bad_mask_shapes(self): + with self.assertRaises(ValueError): + padding_mask = tf.convert_to_tensor([[[1, 1, 0], [1, 0, 0]]]) + attention_mask = tf.convert_to_tensor( + [[0, 0, 1], [0, 1, 0], [1, 0, 0]] + ) + inputs = tf.random.uniform(shape=[1, 3, 2]) + utils.merge_padding_and_attention_mask( + inputs, + padding_mask, + attention_mask, + ) + + with self.assertRaises(ValueError): + padding_mask = tf.convert_to_tensor([[1, 1, 0]]) + attention_mask = tf.convert_to_tensor([[0, 0, 1], [1, 0, 0]]) + inputs = tf.random.uniform(shape=[1, 3, 2]) + utils.merge_padding_and_attention_mask( + inputs, + padding_mask, + attention_mask, + )