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.

Sign up for GitHub

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

Jump to bottom

转换规则 No. 181-184 #195

Merged
merged 2 commits into from
Jul 31, 2023
Merged

转换规则 No. 181-184 #195

merged 2 commits into from
Jul 31, 2023

Conversation

co63oc
Copy link
Contributor

@co63oc co63oc commented Jul 25, 2023

PR Docs

#112

文档 PaddlePaddle/docs#6044

torch.distributions.transforms.SoftmaxTransform 为测试用例使用增加

PR APIs

181 torch.distributions.transformed_distribution.TransformedDistribution
182 torch.distributions.uniform.Uniform
183 torch.distributions.transforms.Transform
184 torch.distributions.transforms.ComposeTransform

@paddle-bot paddle-bot bot added the contributor External developers label Jul 25, 2023
@luotao1 luotao1 added the HappyOpenSource 快乐开源活动issue与PR label Jul 26, 2023
@luotao1 luotao1 mentioned this pull request Jul 26, 2023
Closed
zhwesky2010
zhwesky2010 reviewed Jul 26, 2023
View reviewed changes
Copy link
Collaborator

@zhwesky2010 zhwesky2010 left a comment

Choose a reason for hiding this comment

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

这一系列API路径太长了，建议都实现torch.distributions.*的简洁调用，然后通过 api_alias_mapping.sjon 来配置更长的别名。
对于单测的话只共用一个文件，主要实现简洁的主调用，对于别名加一两个test_alias_case就可以

tests/test_distributions_transformed_distribution_TransformedDistribution.py Outdated
pytorch_code = textwrap.dedent(
"""
import torch
base_distribution = torch.distributions.normal.Normal(torch.tensor([0.0]), torch.tensor([1.0]))
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.

已修改

paconvert/api_mapping.json Outdated
"cache_size": ""
}
},
"torch.distributions.uniform.Uniform": {
Copy link
Collaborator

@zhwesky2010 zhwesky2010 Jul 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

这种API看起来别名很长，可以简洁调用，可以用 api_alias_mapping.json 配置一下

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

zhwesky2010
zhwesky2010 approved these changes Jul 31, 2023
View reviewed changes
tests/test_distributions_Uniform.py
obj.run(pytorch_code, ["result"], check_value=False)


def test_case_3():
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个测试case，看起来和上面那个测试case没什么区别

我先合入了，后面可以看看

Copy link
Contributor Author

Choose a reason for hiding this comment

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

不同的是使用别名和不使用别名
torch.distributions.Uniform
torch.distributions.uniform.Uniform

@zhwesky2010 zhwesky2010 merged commit 27ebdae into PaddlePaddle:master Jul 31, 2023
@co63oc co63oc deleted the api181 branch August 4, 2023 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhwesky2010 zhwesky2010 zhwesky2010 approved these changes

Assignees

@luotao1 luotao1

@zhwesky2010 zhwesky2010

@ROckDog22 ROckDog22

Labels
contributor External developers HappyOpenSource 快乐开源活动issue与PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@co63oc @zhwesky2010 @luotao1 @ROckDog22