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

[Dy2St] pir dy2st unittest verification - Part 2 #58686

Merged
merged 8 commits into from
Nov 7, 2023

Conversation

gouzil
Copy link
Member

@gouzil gouzil commented Nov 4, 2023

PR types

Others

PR changes

Others

Description

开放部分单测测试PIR_API

修复test_legacy_and_pir_api_and_pir_exe不生成单测的问题

修改方法名称:

  • to_pir_ast_test -> to_pir_api_test
  • to_pir_test -> to_pir_exe_test

相关链接:

Copy link

paddle-bot bot commented Nov 4, 2023

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

Comment on lines 103 to 110
def to_pir_api_test(fn):
@wraps(fn)
def impl(*args, **kwargs):
logger.info("[PIR][AST] running pir api")
logger.info("[PIR] running pir api")
ir_outs = None
try:
with paddle.pir_utils.IrGuard():
paddle.disable_static()
ir_outs = fn(*args, **kwargs)
finally:
paddle.enable_static()
with paddle.pir_utils.IrGuard():
paddle.disable_static()
ir_outs = fn(*args, **kwargs)
Copy link
Member

Choose a reason for hiding this comment

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

挪到 to_pir_exe_test 下面,相关代码都按照「老 IR」、「中间态」、「最终态」的顺序作为规范

@wraps(fn)
def impl(*args, **kwargs):
logger.info("[PIR] running pir")
logger.info("[PIR] running pir exe")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
logger.info("[PIR] running pir exe")
logger.info("[PIR_EXE] running pir exe")

@wraps(fn)
def impl(*args, **kwargs):
logger.info("[PIR][AST] running pir api")
logger.info("[PIR] running pir api")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
logger.info("[PIR] running pir api")
logger.info("[PIR_API] running pir api")

@@ -284,12 +281,12 @@ def test_legacy_and_pir(fn):


def test_legacy_and_pir_api(fn):
fn = set_ir_mode(IrMode.LEGACY_IR | IrMode.PIR_API)
fn = set_ir_mode(IrMode.LEGACY_IR | IrMode.PIR_API)(fn)
return fn


def test_legacy_and_pir_api_and_pir_exe(fn):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def test_legacy_and_pir_api_and_pir_exe(fn):
def test_legacy_and_pir_exe_and_pir_api(fn):

@@ -360,7 +359,7 @@ def test_to_static(self):
net = ListWithCondNet()
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
Member Author

Choose a reason for hiding this comment

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

是的这个还不能跑

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
Member

Choose a reason for hiding this comment

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

这个单测看起来只是测试 utils 功能函数的,所以与 IR 是无关的

但为了保险,我们测一下 legacy 和最终态吧,中间态没必要测

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@PaddlePaddle PaddlePaddle locked and limited conversation to collaborators Nov 6, 2023
@PaddlePaddle PaddlePaddle unlocked this conversation Nov 6, 2023
@SigureMo SigureMo closed this Nov 7, 2023
@SigureMo SigureMo reopened this Nov 7, 2023
Copy link
Member

@SigureMo SigureMo left a comment

Choose a reason for hiding this comment

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

LGTMeow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants