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] 解决废弃算子mul op 在新 IR 下的translator问题 #56550

Merged
merged 33 commits into from
Oct 30, 2023
Merged

[PIR] 解决废弃算子mul op 在新 IR 下的translator问题 #56550

merged 33 commits into from
Oct 30, 2023

Conversation

DrRyanHuang
Copy link
Member

PR types

Others

PR changes

Others

Description

Others

@paddle-bot paddle-bot bot added the contributor External developers label Aug 22, 2023
@DrRyanHuang DrRyanHuang marked this pull request as draft August 23, 2023 00:53
@DrRyanHuang DrRyanHuang marked this pull request as ready for review August 29, 2023 16:00
@DrRyanHuang

This comment was marked as off-topic.

@kangguangli
Copy link
Contributor

@kangguangli 非常不好意思,打扰您了

Op mul arg out should be optional if it can be empty

想问一下您,我是需要重写一下 OpTranscriber::GenerateOperationOutput 方法吗?

把这一行注释掉嘛?

      IR_ENFORCE(info.optional,
                 "Op %s arg %s should be optional if it can be empty",
                 op_desc.Type(),
                 legacy_output_name);
2023-09-05 21:26:30 ======================================================================
2023-09-05 21:26:30 ERROR: test_check_output (test_mul_op.TestMulOp2)
2023-09-05 21:26:30 ----------------------------------------------------------------------
2023-09-05 21:26:30 Traceback (most recent call last):
2023-09-05 21:26:30   File "/workspace/Paddle/build/test/legacy_test/test_mul_op.py", line 90, in test_check_output
2023-09-05 21:26:30     self.check_output(check_dygraph=False)
2023-09-05 21:26:30   File "/workspace/Paddle/build/python/eager_op_test.py", line 2605, in check_output
2023-09-05 21:26:30     res = self.check_output_with_place(
2023-09-05 21:26:30   File "/workspace/Paddle/build/python/eager_op_test.py", line 2459, in check_output_with_place
2023-09-05 21:26:30     static_checker.check()
2023-09-05 21:26:30   File "/workspace/Paddle/build/python/eager_op_test.py", line 2117, in check
2023-09-05 21:26:30     self.calculate_output()
2023-09-05 21:26:30   File "/workspace/Paddle/build/python/eager_op_test.py", line 2125, in calculate_output
2023-09-05 21:26:30     outs, fetch_list = self.op_test._calc_output(
2023-09-05 21:26:30   File "/workspace/Paddle/build/python/eager_op_test.py", line 1513, in _calc_output
2023-09-05 21:26:30     self._check_ir_output(place, program, feed_map, fetch_list, outs)
2023-09-05 21:26:30   File "/workspace/Paddle/build/python/eager_op_test.py", line 1395, in _check_ir_output
2023-09-05 21:26:30     ir_outs = executor.run(
2023-09-05 21:26:30   File "/workspace/Paddle/build/python/paddle/fluid/executor.py", line 1635, in run
2023-09-05 21:26:30     res = self._run_impl(
2023-09-05 21:26:30   File "/workspace/Paddle/build/python/paddle/fluid/executor.py", line 1812, in _run_impl
2023-09-05 21:26:30     program, new_exe = self._executor_cache.get_program_and_executor(
2023-09-05 21:26:30   File "/workspace/Paddle/build/python/paddle/fluid/executor.py", line 890, in get_program_and_executor
2023-09-05 21:26:30     return self._get_cached_program_and_executor(
2023-09-05 21:26:30   File "/workspace/Paddle/build/python/paddle/fluid/executor.py", line 1011, in _get_program_and_executor
2023-09-05 21:26:30     new_exe = _StandaloneExecutor(place, plan, scope)
2023-09-05 21:26:30   File "/workspace/Paddle/build/python/paddle/fluid/executor.py", line 787, in __init__
2023-09-05 21:26:30     self._new_exe = self._create_new_executor()
2023-09-05 21:26:30   File "/workspace/Paddle/build/python/paddle/fluid/executor.py", line 811, in _create_new_executor
2023-09-05 21:26:30     new_exe = core.StandaloneExecutor(self._place, self._plan, self._scope)
2023-09-05 21:26:30 RuntimeError: Error occured at: ../paddle/fluid/ir_adaptor/translator/op_translator.cc:434 :
2023-09-05 21:26:30 Op mul arg out should be optional if it can be empty

@DrRyanHuang
这种情况理论上需要添加参数映射,错误原因是PT模块安装OpYamlInfo从OpDesc中查找需要的信息,OpYamlInfo标记的输出是out,但是实际上OpDesc中存储的是Out,我们需要标记这种映射关系。

可以尝试修改op_compat.yaml,如下:

- op : matmul_with_flatten (mul)
  backward : matmul_with_flatten_grad (mul_grad) 
  inputs :
    {x : X, y : Y}
  outputs :
    out : Out
  extra :
    attrs : [bool use_mkldnn = false, float scale_x = 1.0f, 'float[] scale_y = {1.0f}',
             float scale_out = 1.0f, bool force_fp32_output = false]

这里写的可以参考下。我们对mul的处理比较特殊,如果这样后续有问题的话,可以再@下我。

@paddle-ci-bot
Copy link

paddle-ci-bot bot commented Sep 19, 2023

Sorry to inform you that ebce876's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

@DrRyanHuang

This comment was marked as off-topic.

@paddle-ci-bot
Copy link

paddle-ci-bot bot commented Oct 4, 2023

Sorry to inform you that 51b9e8c's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

@paddle-ci-bot
Copy link

paddle-ci-bot bot commented Oct 24, 2023

Sorry to inform you that 62b9089's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

@DrRyanHuang

This comment was marked as off-topic.

@DrRyanHuang DrRyanHuang marked this pull request as draft October 27, 2023 16:42
@DrRyanHuang DrRyanHuang marked this pull request as ready for review October 30, 2023 05:08
Copy link
Contributor

@Aurelius84 Aurelius84 left a comment

Choose a reason for hiding this comment

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

comment 里的建议下个PR 统一fix下~~

Copy link
Contributor

@kangguangli kangguangli left a comment

Choose a reason for hiding this comment

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

LGTM overall. But there're two problems that we should consider.

  1. insert two much reshape, every pair of mul/mul_grad will lead to 6-10 additional reshape op insertions, is there some way to decrease this number?
  2. too much redundant code. Try to compress by some kind of abstraction like function.

Copy link
Contributor

@zyfncg zyfncg left a comment

Choose a reason for hiding this comment

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

LGTM for op_compat

@Aurelius84 Aurelius84 merged commit 119f100 into PaddlePaddle:develop Oct 30, 2023
28 checks passed
@DrRyanHuang DrRyanHuang deleted the first_IR branch October 30, 2023 12:08
zeroRains pushed a commit to zeroRains/Paddle that referenced this pull request Nov 8, 2023
danleifeng pushed a commit to danleifeng/Paddle that referenced this pull request Nov 14, 2023
@DrRyanHuang DrRyanHuang changed the title 解决废弃算子mul op 在新 IR 下的translator问题 [PIR] 解决废弃算子mul op 在新 IR 下的translator问题 Nov 21, 2023
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.

4 participants