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

[ConvBert P0 P1 P2] Add PretrainedConfig, unit tests and input_embs #5886

Merged
merged 4 commits into from
May 13, 2023

Conversation

KB-Ding
Copy link
Contributor

@KB-Ding KB-Ding commented May 10, 2023

PR types

PR changes

Description

@paddle-bot
Copy link

paddle-bot bot commented May 10, 2023

Thanks for your contribution!

@codecov
Copy link

codecov bot commented May 10, 2023

Codecov Report

Merging #5886 (4739901) into develop (c3da2fe) will increase coverage by 0.42%.
The diff coverage is 95.57%.

@@             Coverage Diff             @@
##           develop    #5886      +/-   ##
===========================================
+ Coverage    61.94%   62.36%   +0.42%     
===========================================
  Files          491      491              
  Lines        69136    69245     +109     
===========================================
+ Hits         42824    43188     +364     
+ Misses       26312    26057     -255     
Impacted Files Coverage Δ
paddlenlp/transformers/convbert/tokenizer.py 100.00% <ø> (ø)
paddlenlp/transformers/convbert/modeling.py 85.62% <94.87%> (+64.85%) ⬆️
paddlenlp/transformers/__init__.py 100.00% <100.00%> (ø)
paddlenlp/transformers/convbert/configuration.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

Examples:

```python
>>> from paddlenlp.transformers import BertModel, BertConfig
Copy link
Member

Choose a reason for hiding this comment

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

这里的示例可以修改成convbert

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

@@ -0,0 +1,307 @@
# Copyright (c) 2022 PaddlePaddle Authors. All Rights Reserved.
Copy link
Member

Choose a reason for hiding this comment

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

这里的年份可以统一改下,下同

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

Copy link
Collaborator

@sijunhe sijunhe left a comment

Choose a reason for hiding this comment

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

完成得挺好的,小地方修一下就好

Comment on lines 15 to 16
from .modeling import *
from .tokenizer import *
# from .modeling import *
# from .tokenizer import *
Copy link
Collaborator

Choose a reason for hiding this comment

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

直接删除

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

Comment on lines 767 to 776
See :class:`ConvBertModel`.
output_hidden_states (bool, optional):
Whether to return the hidden states of all layers.
Defaults to `False`.
output_attentions (bool, optional):
Whether to return the attentions tensors of all attention layers.
Defaults to `False`.
return_dict (bool, optional):
Whether to return a :class:`~paddlenlp.transformers.model_outputs.QuestionAnsweringModelOutput` object. If
`False`, the output will be a tuple of tensors. Defaults to `False`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

这些应该也相应得补到ConvBertModel的注释里去

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已添加,此处均改为 “See :class:ConvBertModel.”



@parameterized_class(
("return_dict", "use_labels", "use_inputs_embeds"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

use_inputs_embeds 可以从parameterized里去了,在class定义里面直接打开use_inputs_embeds=True就好了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已删去,设置 use_test_inputs_embeds: bool = True。

Copy link
Collaborator

@sijunhe sijunhe left a comment

Choose a reason for hiding this comment

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

great job. LGTM

@sijunhe sijunhe merged commit 0a48644 into PaddlePaddle:develop May 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants