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

[PIR+CINN]Part-2 Pybind IrParser.ParseProgram and Polish UT into check_run #59449

Merged
merged 6 commits into from Nov 30, 2023

Conversation

Aurelius84
Copy link
Contributor

@Aurelius84 Aurelius84 commented Nov 28, 2023

PR types

New features

PR changes

Others

Description

Pcard-67164
前置PR见:#59353

  • 在Pybind层暴露了 paddle.pir.parse_program(str) 接口,支持前端从string里反序列化得到Program对象
  • 升级了test_subgraph_exporter 单测,由静态check优化为加载file文件,反序列化并执行Program
  • 此PR验证了 CINN 子图抽取、导出、反序列化、执行全流程

Copy link

paddle-bot bot commented Nov 28, 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.

add unittest

fix UT not take effect

[PIR+CINN]Pybind IrParser.ParseProgram and Polish UT into check_run

fix cmake flasgs

remove VLOG

fix code comment
self.assertEqual(len(outs), 0)


# class TestSaveInferProg(TestSaveFwdBwdProg):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个依赖动转静SOT一个BUG Fix 的PR,待依赖PR合入后,单独打开

@Aurelius84 Aurelius84 changed the title [PIR+CINN]Pybind IrParser.ParseProgram and Polish UT into check_run [PIR+CINN]Part-2 Pybind IrParser.ParseProgram and Polish UT into check_run Nov 29, 2023
phlrain
phlrain previously approved these changes Nov 29, 2023
Comment on lines +437 to +447
need_fetch_info = []
for i, fetch_var in enumerate(fetch_targets):
if isinstance(fetch_var, str):
if fetch_var not in fetch_info[1]:
raise Exception(
f"Found fetch_target[{i}] is type(str) and doesn't have fetch op."
)
elif fetch_var not in fetch_info[0]:
need_fetch_info.append(fetch_var)

return need_fetch_info
Copy link
Contributor

Choose a reason for hiding this comment

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

这段逻辑删除了对之前fetch op数量报错以及警告的检查,改为补充缺失的fetch op,会不会存在这样的问题,用户多次调用executor run,后续run可能和前边的run并没有太大关系,但是fetch op依然滞留到了program里,导致跑后续run的过程中,依然会fetch并不需要fetch的数据,由于fetch会进行copy操作,会造成隐形性能开销,这里是不是还是拦截或者提示一下相关信息比较好

Copy link
Contributor Author

Choose a reason for hiding this comment

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

之前的逻辑这里也会给program添加缺失的fetch_ops,这个行为是没有改变的,而且之前的add_pir_fetch_ops是会对所有的fetch_list 添加,即使之前部分fetch_var已经有fetch_op了。
关于多次run且彼此之间的fetch_list不一样的问题,其实在run()接口里应该要先clone program,然后add_feed_fetch_ops,然后缓存起来。后续优先根据feed/fetch 信息来查缓存program,这样每次run就是独立的,互不影响。

这个cache策略是已有的,在上层做的,add_feed_fetch_ops 是不需要关心这个缓存逻辑的

@Aurelius84 Aurelius84 merged commit 09401e6 into PaddlePaddle:develop Nov 30, 2023
29 of 30 checks passed
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.

None yet

3 participants