[PIR] Refactor pir `GetNameMap` and assert `value` unique #66738

gouzil · 2024-07-29T09:08:50Z

PR Category

Performance Optimization

PR Types

Not User Facing

Description

清除GetNameMap
添加检查，确保一个 Value 只有一个 name

相关链接:

paddle-bot · 2024-07-29T09:08:56Z

你的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.

SigureMo · 2024-07-31T07:45:26Z

paddle/fluid/eager/to_static/run_program_op_node.h

@@ -495,6 +494,29 @@ inline void PirRunProgramAPI(
    details::print_collection(skip_names_set);
    interpreter_core->SetSkipGcVars(skip_names_set);

+    // check only one name


可以考虑：

在 GetNameFromValue 里加，这些 name 会直接用于 share 到 scope，所以这里能确保唯一不会有任何问题

unify_value_names 跑完后对所有 Value 重新检查一下，这里的 Program 之后也会直接传给 RunProgramOp，这里保证唯一也基本不会有啥问题

SigureMo · 2024-07-31T11:37:36Z

python/paddle/jit/dy2static/pir_partial_program.py

@@ -326,6 +326,8 @@ def program_attr(self):
        rename_mapping = RunnableProgram.unify_value_names(
            self.backward_program, rename_mapping
        )
+        # check if every value has only one name.
+        assert len(rename_mapping.values()) == len(set(rename_mapping.keys()))


？这是在干啥呢？

SigureMo · 2024-07-31T11:41:44Z

python/paddle/jit/dy2static/pir_partial_program.py

@@ -326,6 +326,8 @@ def program_attr(self):
        rename_mapping = RunnableProgram.unify_value_names(
            self.backward_program, rename_mapping
        )
+        # check if every value has only one name.
+        assert len(rename_mapping.values()) == len(set(rename_mapping.values()))


？这又是在干啥呢？

SigureMo · 2024-07-31T12:06:53Z

python/paddle/jit/dy2static/pir_partial_program.py

@@ -327,7 +327,8 @@ def program_attr(self):
            self.backward_program, rename_mapping
        )
        # check if every value has only one name.
-        assert len(rename_mapping.values()) == len(set(rename_mapping.values()))
+        for value in rename_mapping.values():


会挂，有的 value 被删了，需要重新获取全部 values

SigureMo · 2024-07-31T12:40:00Z

paddle/fluid/pir/utils/name_analysis.h

@@ -39,6 +40,11 @@ std::vector<std::string> GetValueOutputNames(pir::Value value);
 pir::Value GetOutputValueByName(const pir::Program &program,
                                const std::string &name);

+inline bool IsOnlyOneValueName(pir::Value value) {


Suggested change

inline bool IsOnlyOneValueName(pir::Value value) {

inline bool HasOnlyOneValueName(pir::Value value) {

SigureMo

LGTMeow

SigureMo · 2024-07-31T15:41:15Z

paddle/fluid/eager/to_static/run_program_op_node.h

+  // check only one name
+  for (auto value : values) {
+    PADDLE_ENFORCE(pir::utils::name_analysis::HasOnlyOneValueName(value),
+                   "Value should have only one name");


2024-07-31 22:53:55 **************** 2024-07-31 22:53:55 0. The error message you wrote in PADDLE_ENFORCE{_**} or PADDLE_THROW does not meet our error message writing specification. Possible errors include 1. the error message is empty / 2. the error message is too short / 3. the error type is not specified. Please read the specification [ https://github.com/PaddlePaddle/Paddle/wiki/Paddle-Error-Message-Writing-Specification ], then refine the error message. If it is a mismatch, please request luotao1 or Aurelius84 review and approve. 2024-07-31 22:53:55 The PADDLE_ENFORCE{_**} or PADDLE_THROW entries that do not meet the specification are as follows: 2024-07-31 22:53:55 PADDLE_ENFORCE(pir::utils::name_analysis::HasOnlyOneValueName(value), + "Value should have only one name"); 2024-07-31 22:53:55 There are 1 approved errors. 2024-07-31 22:53:55 ****************

需要声明具体类型，不能只写 message

我加一下吧

啊……我说的是二选一，你两个地方都加了啊……这里我直接删了

[PIR] rm GetNameMap

4888a31

paddle-bot bot added the contributor External developers label Jul 29, 2024

gouzil added 3 commits July 29, 2024 19:20

fix ci

43210b2

add check name

17c914c

fix ci

60c9619

gouzil requested a review from SigureMo July 30, 2024 15:32

clean code

eb9697b

SigureMo reviewed Jul 31, 2024

View reviewed changes

fix review

18ed466

SigureMo reviewed Jul 31, 2024

View reviewed changes

fix

e8e861e

SigureMo reviewed Jul 31, 2024

View reviewed changes

fix review

3f031aa

SigureMo reviewed Jul 31, 2024

View reviewed changes

unify_value_names add check value name

073c658

SigureMo reviewed Jul 31, 2024

View reviewed changes

gouzil and others added 3 commits July 31, 2024 20:47

rename is_only_one_name -> has_only_one_name

06c8c24

Merge branch 'develop' into pir/reconfiguration_getname_3

3f92536

make _has_only_one_name a method

f180839

SigureMo previously approved these changes Jul 31, 2024

View reviewed changes

SigureMo changed the title ~~[WIP][PIR] Refactor pir GetNameMap and assert value unique~~ [PIR] Refactor pir GetNameMap and assert value unique Jul 31, 2024

SigureMo reviewed Jul 31, 2024

View reviewed changes

remove dup check

b47a023

SigureMo dismissed their stale review via b47a023 July 31, 2024 15:46

SigureMo approved these changes Jul 31, 2024

View reviewed changes

luotao1 assigned wanghuancoder and unassigned wanghuancoder Aug 1, 2024

SigureMo merged commit 3454ecc into PaddlePaddle:develop Aug 1, 2024
30 of 31 checks passed

gouzil deleted the pir/reconfiguration_getname_3 branch November 22, 2024 15:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[PIR] Refactor pir `GetNameMap` and assert `value` unique #66738

[PIR] Refactor pir `GetNameMap` and assert `value` unique #66738

Uh oh!

gouzil commented Jul 29, 2024 •

edited

Loading

Uh oh!

paddle-bot bot commented Jul 29, 2024

Uh oh!

SigureMo Jul 31, 2024

Uh oh!

SigureMo Jul 31, 2024

Uh oh!

SigureMo Jul 31, 2024

Uh oh!

SigureMo Jul 31, 2024

Uh oh!

SigureMo Jul 31, 2024

Uh oh!

SigureMo left a comment

Uh oh!

SigureMo Jul 31, 2024

Uh oh!

SigureMo Jul 31, 2024

Uh oh!

SigureMo Jul 31, 2024

Uh oh!

Uh oh!

Uh oh!

	inline bool IsOnlyOneValueName(pir::Value value) {
	inline bool HasOnlyOneValueName(pir::Value value) {

[PIR] Refactor pir GetNameMap and assert value unique #66738

[PIR] Refactor pir GetNameMap and assert value unique #66738

Uh oh!

Conversation

gouzil commented Jul 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Category

PR Types

Description

Uh oh!

paddle-bot bot commented Jul 29, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SigureMo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

[PIR] Refactor pir `GetNameMap` and assert `value` unique #66738

[PIR] Refactor pir `GetNameMap` and assert `value` unique #66738

gouzil commented Jul 29, 2024 •

edited

Loading