Skip to content

Navigation Menu

Sign in
Sign up
This repository was archived by the owner on Jan 21, 2025. It is now read-only.

Add scores for generated text in inference mode - #164

Open
allen-q wants to merge 5 commits into
tensorflow:master from
allen-q:Add_probability_in_inference_mode
Open

Add scores for generated text in inference mode #164
allen-q wants to merge 5 commits into
tensorflow:master from
allen-q:Add_probability_in_inference_mode

Conversation

@allen-q

@allen-q allen-q commented Aug 20, 2020
edited
Loading

Copy link
Copy Markdown

Background:

I was using the T5 model and wanted to get the scores at inference mode along with the generated text. However, this feature is not supported by T5 at the moment and I was advised to implement this feature and raise a pull request. Please see google-research/text-to-text-transfer-transformer#311. for more details.

This PR implemented this function to add the scores(log likelihood) along the generated text in the outputs when a model is exported in SavedModel format.

Changed file:
./mesh/mesh_tensorflow/transformer/utils.py

SignatureDef Diff
Below is how a T5 MTF SavedModel SignatureDef looks like before the change:

The given SavedModel SignatureDef contains the following input(s):
 inputs['input'] tensor_info:
 dtype: DT_STRING
 shape: (-1)
 name: inputs:0
The given SavedModel SignatureDef contains the following output(s):
 outputs['inputs'] tensor_info:
 dtype: DT_STRING
 shape: (10)
 name: SentenceTokenizer/SentenceTokenizer/SentencepieceDetokenizeOp:0
 outputs['outputs'] tensor_info:
 dtype: DT_STRING
 shape: (10)
 name: SentenceTokenizer_1/SentenceTokenizer/SentencepieceDetokenizeOp:0
Method name is: tensorflow/serving/predict

Below is how a T5 MTF SavedModel SignatureDef looks like after the change:

The given SavedModel SignatureDef contains the following input(s):
 inputs['input'] tensor_info:
 dtype: DT_STRING
 shape: (-1)
 name: inputs:0
The given SavedModel SignatureDef contains the following output(s):
 outputs['inputs'] tensor_info:
 dtype: DT_STRING
 shape: (10)
 name: SentenceTokenizer/SentenceTokenizer/SentencepieceDetokenizeOp:0
 outputs['outputs'] tensor_info:
 dtype: DT_STRING
 shape: (10)
 name: SentenceTokenizer_1/SentenceTokenizer/SentencepieceDetokenizeOp:0
 outputs['scores'] tensor_info:
 dtype: DT_FLOAT
 shape: (10)
 name: reshape_17/parallel_0/Reshape:0
Method name is: tensorflow/serving/predict

Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

i️ Googlers: Go here for more info.

allen-q force-pushed the Add_probability_in_inference_mode branch from a3d42a3 to 7a96222 Compare August 31, 2020 11:00

Copy link
Copy Markdown

CLAs look good, thanks!

i️ Googlers: Go here for more info.

...nsistent with other scores. create a compute_score function to remove duplicate code.
@allen-q allen-q changed the title (削除) Add probabilities for generated text in inference model (削除ここまで) (追記) Add scores for generated text in inference model (追記ここまで) Aug 31, 2020
@allen-q allen-q changed the title (削除) Add scores for generated text in inference model (削除ここまで) (追記) Add scores for generated text in inference mode (追記ここまで) Aug 31, 2020
Comment thread mesh_tensorflow/transformer/utils.py Outdated

@adarob adarob left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, this is going to result in an approximate doubling of inference time. Can you make it so the score is computed in sample_autoregressive?

adarob commented Oct 2, 2020

Copy link
Copy Markdown
Member

@allen-q do you plan on following up with this? thanks!

allen-q commented Oct 6, 2020 via email

Copy link
Copy Markdown
Author
Sorry for the late reply. I'm still planning to make it work but probably won't have time in the next couple of weeks. Happy for someone to take a look at it in the meantime.
...
On Sat, 3 Oct 2020, 12:06 am Adam Roberts, ***@***.***> wrote: @allen-q <https://github.com/allen-q> do you plan on following up with this? thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#164 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AE6GK65MHIRVRNBEVT533DTSIXM7DANCNFSM4QF6TH5A> .

adarob commented Oct 16, 2020

Copy link
Copy Markdown
Member

No worries. Perhaps we can just gate this with a bool arg for now until we have the "free" version?

marton-avrios commented Dec 10, 2020
edited
Loading

Copy link
Copy Markdown

I am currently working on a return_logits option for sample_autoregressive that just returns the already available logits together with outputs so no extra computation is involved. If this is set to True it returns an (outputs, output_logits) tuple instead of outputs. But I think it makes no sense to return only outputs anymore so not sure another argument should be introduced.

bzz reacted with thumbs up emoji

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers
@daphnei daphnei daphnei left review comments
@adarob adarob adarob requested changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /