Added an Example for BLEU. #799
Conversation
I have made some changes in the docstring example of the TokenAndPosition Embedding Layer as suggested in issue number 658.
I have changed the epsilon value 1e-5 to 1e-12.
I have changed the epsilon value from 1e-5 to 1e-12.
I would like to inform you that I have made some updates to the content. Firstly, I have added an example for BLEU which was not present before. This new example serves as an illustration of the concept and should be helpful for those who are new to this topic. Furthermore, I have also fixed the URLs by wrapping them in markdown. This should make it easier for readers to access the resources mentioned in the content.
keras_nlp/metrics/bleu.py
Outdated
| Examples: | ||
| ```python | ||
| bleu = keras_nlp.metrics.Bleu(max_order=4) | ||
| # reference sentence | ||
| ref_sentence = "the quick brown fox jumps over the lazy dog" | ||
| # predicted sentence | ||
| pred_sentence = "the quick brown fox jumps over the box" | ||
| # compute BLEU score | ||
| score = bleu([ref_sentence], [pred_sentence]) | ||
| print("BLEU score:", score) | ||
| ``` |
There was a problem hiding this comment.
Oh, shoot! I must have forgotten to add examples. Instead of fenced doc-strings, can we have examples this way: https://github.com/keras-team/keras-nlp/blob/master/keras_nlp/metrics/rouge_l.py#L46-L113?
keras_nlp/metrics/bleu.py
Outdated
| penalise short predictions. For more details, see the following article: | ||
| https://cloud.google.com/translate/automl/docs/evaluate#bleu. | ||
| [Link](https://cloud.google.com/translate/automl/docs/evaluate#bleu). |
There was a problem hiding this comment.
We can probably change this to
For more details, see [this article](https://cloud.google.com/translate/automl/docs/evaluate#bleu).
keras_nlp/metrics/bleu.py
Outdated
| `"tokenizer_13a"` tokenizer | ||
| (https://github.com/mjpost/sacrebleu/blob/v2.1.0/sacrebleu/tokenizers/tokenizer_13a.py). | ||
| [Link](https://github.com/mjpost/sacrebleu/blob/v2.1.0/sacrebleu/tokenizers/tokenizer_13a.py). |
There was a problem hiding this comment.
Here, as well, we can probably change this to
...
[SacreBLEU's `"tokenizer_13a"` tokenizer](https://github.com/mjpost/sacrebleu/blob/v2.1.0/sacrebleu/tokenizers/tokenizer_13a.py).
| @@ -140,7 +140,7 @@ def __init__( | |||
| x = keras.layers.LayerNormalization( | |||
| name="encoder_embeddings_layer_norm", | |||
| axis=-1, | |||
| epsilon=1e-5, | |||
| epsilon=1e-12, | |||
There was a problem hiding this comment.
Why are we changing the epsilon values here? BART uses epsilon=1e-5: https://github.com/huggingface/transformers/blob/main/src/transformers/models/bart/modeling_bart.py#L732. The PyTorch default is 1e-5: https://pytorch.org/docs/stable/generated/torch.nn.LayerNorm.html.
There was a problem hiding this comment.
I have read an issue about changing the epsilon value, that's why I changed it:
#642
I will update it as you said from 1e-12 to 1e-5.
Changed the epsilon value from 1e-12 to 1e-5.
Made the changes as suggested by the reviewer.
There was a problem hiding this comment.
Thanks for the update, @Neeshamraghav012! A few minor comments, and one major comment about adding more examples.
Oh, and please run the code formatters (./shell/format.sh).
keras_nlp/metrics/bleu.py
Outdated
| Examples: | ||
|
|
||
| 1. Calculate BLEU score by calling Bleu directly. | ||
| >>> bleu = keras_nlp.metrics.Bleu(max_order=4) | ||
| >>> ref_sentence = "the quick brown fox jumps over the lazy dog" | ||
| >>> pred_sentence = "the quick brown fox jumps over the box" | ||
| >>> score = bleu([ref_sentence], [pred_sentence]) | ||
| <tf.Tensor(0.7420885, shape=(), dtype=float32)> |
There was a problem hiding this comment.
No need for the extra indentation, it can be in line with "Examples:!
| **kwargs: Other keyword arguments. | ||
|
|
||
| **kwargs: Other keyword arguments. | ||
| Examples: |
There was a problem hiding this comment.
We need to add more examples, which cover the various input types and input shapes. Check this for reference: https://github.com/keras-team/keras-nlp/blob/master/keras_nlp/metrics/rouge_l.py#L46.
We also need to demonstrate that we can pass multiple references for the same sample, i.e., (references, translation) pair. We need a separate example for this.
keras_nlp/metrics/bleu.py
Outdated
| @@ -92,8 +92,15 @@ class Bleu(keras.metrics.Metric): | |||
| dtype: string or tf.dtypes.Dtype. Precision of metric computation. If | |||
| not specified, it defaults to tf.float32. | |||
| name: string. Name of the metric instance. | |||
| **kwargs: Other keyword arguments. | |||
|
|
|||
| **kwargs: Other keyword arguments. | |||
There was a problem hiding this comment.
Let's remove **kwargs...we don't actually document it anywhere in the library. And please add a newline after the last arg!
keras_nlp/metrics/bleu.py
Outdated
| `"tokenizer_13a"` tokenizer, see | ||
| [tokenizer details](https://github.com/mjpost/sacrebleu/blob/v2.1.0/sacrebleu/tokenizers/tokenizer_13a.py). |
There was a problem hiding this comment.
This didn't work?
[SacreBLEU's `"tokenizer_13a"` tokenizer](https://github.com/mjpost/sacrebleu/blob/v2.1.0/sacrebleu/tokenizers/tokenizer_13a.py).
I think this looks cleaner, what do you think?
I have added more example as suggested by the reviewer, and made some minor changes as @abheesht17 said.
I would like to inform you that I have made some updates to the content. Firstly, I have added an example for BLEU which was not present before. This new example serves as an illustration of the concept and should be helpful for those who are new to this topic.
Furthermore, I have also fixed the URLs by wrapping them in markdown. This should make it easier for readers to access the resources mentioned in the content.