Skip to content
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

Clean text.py and decode.py for API 2.0 #26853

Merged
merged 29 commits into from
Oct 16, 2020

Conversation

guoshengCS
Copy link
Contributor

@guoshengCS guoshengCS commented Sep 1, 2020

PR types

New features

PR changes

APIs

Describe

  1. 删除text.py
  2. 整理解码相关API,增加paddle.fluid.layers.rnn下dynamic_decode的动态图支持,并将其与BeamSearchDecoder导入paddle.nn.decode下,将gather_tree导入paddle.nn.functional下

@paddle-bot-old
Copy link

paddle-bot-old bot commented Sep 1, 2020

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@LiuChiachi
Copy link
Contributor

LiuChiachi commented Sep 30, 2020

该PR完成了:
1.对dynamic_decode添加了对动态图的支持
2.原paddle.text及paddle.nn下相关API删除(相关单元测试也一并删除)、目录迁移,具体方案及原因见下表
image

@guoshengCS guoshengCS changed the title Make dynamic_decode support dygraph and expose to API 2.0 Clean text.py and decode.py for API 2.0 Oct 10, 2020
weight_linear = paddle.to_tensor(
np.array(np.random.rand(32, 32), dtype=paddle.get_default_dtype()))
trg_embeder = lambda x: F.embedding(x, weight=weight, padding_idx=0)
output_layer = lambda x: F.linear(x, weight=weight_linear)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

是否将这里换成nn.Embeddingnn.Linear会更直观一些

Copy link
Contributor

Choose a reason for hiding this comment

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

Done. Thanks

np.array(np.random.rand(32, 32), dtype=paddle.get_default_dtype()))
trg_embeder = lambda x: F.embedding(x, weight=weight, padding_idx=0)
output_layer = lambda x: F.linear(x, weight=weight_linear)
decoder_cell = GRUCell(input_size=32, hidden_size=32)
decoder = BeamSearchDecoder(decoder_cell,
start_token=0,
end_token=1,
beam_size=4,
embedding_fn=trg_embeder,
output_fn=output_layer)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

embedding_fnoutput_fn是否需要调整参数命名

when using fluid.layers.embedding, must unsqueeze in embedding_fn.**
**Note that paddle.nn.functional.embedding should be used here rather than
paddle.nn.Embedding, since shape of ids is [batch_size, beam_size].
when using paddle.nn.Embedding, must unsqueeze in embedding_fn.**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

请检查确认这里的正确性,paddle.nn.Embedding接受的输入的形状应该是和paddle.nn.functional.embedding一样的

output_time_major=False,
impute_finished=False,
is_test=False,
return_length=False,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

可以考虑API中的return_length是否可以去掉

would be called once after the decoding loop.

Parameters:
decoder(Decoder): An instance of `Decoder`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

是否需要将BeamSearchDecoder的基类Decoder也暴露出来

ZeyuChen
ZeyuChen previously approved these changes Oct 10, 2020
Copy link
Member

@ZeyuChen ZeyuChen left a comment

Choose a reason for hiding this comment

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

LGTM

@ZeyuChen ZeyuChen self-requested a review October 10, 2020 07:02
kolinwei
kolinwei previously approved these changes Oct 14, 2020
ZeyuChen
ZeyuChen previously approved these changes Oct 14, 2020
Copy link
Member

@ZeyuChen ZeyuChen left a comment

Choose a reason for hiding this comment

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

LGTM

decoder_cell = GRUCell(hidden_size=128)
import numpy as np
import paddle
from paddle.fluid.layers import BeamSearchDecoder, dynamic_decode

Choose a reason for hiding this comment

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

替换为paddle.nn下面的API

Copy link
Contributor

Choose a reason for hiding this comment

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

Done. Thanks!

saxon-zh
saxon-zh previously approved these changes Oct 14, 2020
Copy link

@saxon-zh saxon-zh left a comment

Choose a reason for hiding this comment

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

LGTM

XiaoguangHu01
XiaoguangHu01 previously approved these changes Oct 14, 2020
Copy link
Contributor

@XiaoguangHu01 XiaoguangHu01 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@jzhang533 jzhang533 left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link

@saxon-zh saxon-zh left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@XiaoguangHu01 XiaoguangHu01 left a comment

Choose a reason for hiding this comment

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

LGTM

@guoshengCS guoshengCS merged commit 0133581 into PaddlePaddle:develop Oct 16, 2020
LiuChiachi added a commit to LiuChiachi/Paddle that referenced this pull request Oct 16, 2020
* Make dynamic_decode support dygraph and expose to API 2.0
test=develop

* update info about BeamSearchDecoder and dynamic_decode

* remove all APIs in paddle.text, expose BeamSearchDecoder and dynamic_decode

* update example code

* delete test_text.py, decode.py, update some doc, fix example code float64

* delete decode import from paddle.nn

* fix unittest bugs

* use dygraph.Embedding instead of nn.Embedding, add paddle.enbale_static()

* update, correct doc

* move dynamic_decode, BeamSearchDecoder API to paddle.nn

* fix code style

* update unittest param, delete import pf text.py

* set dtype of beamsearchtest float64

* update example code of BeamSearchDecoder, dynamic_decode

Co-authored-by: LiuChiaChi <[email protected]>
guoshengCS added a commit that referenced this pull request Oct 17, 2020
* Make dynamic_decode support dygraph and expose to API 2.0
test=develop

* update info about BeamSearchDecoder and dynamic_decode

* remove all APIs in paddle.text, expose BeamSearchDecoder and dynamic_decode

* update example code

* delete test_text.py, decode.py, update some doc, fix example code float64

* delete decode import from paddle.nn

* fix unittest bugs

* use dygraph.Embedding instead of nn.Embedding, add paddle.enbale_static()

* update, correct doc

* move dynamic_decode, BeamSearchDecoder API to paddle.nn

* fix code style

* update unittest param, delete import pf text.py

* set dtype of beamsearchtest float64

* update example code of BeamSearchDecoder, dynamic_decode

Co-authored-by: LiuChiaChi <[email protected]>

Co-authored-by: Guo Sheng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants