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

Zq/add specified autocompare #785

Draft
wants to merge 30 commits into
base: main
Choose a base branch
from

Conversation

NeosZhang
Copy link
Collaborator

@NeosZhang NeosZhang commented Apr 19, 2024

重构autocompare,autocompare功能的开启或关闭不再依赖编译,和DIPU_AUTOCOMPARE_OPS_LIST来设置autocompare功能,可以指定算子进行autocompare。

  • 重构后的wrapped_code代码生成逻辑改变,为所有未fallback的算子生成普通版和autocompare版
image
  • 现在环境变量控制的是注册过程,根据DIPU_AUTOCOMPARE_OPS_LIST来确定注册普通版还是autocomapre版。

dipu/QuickStart.md Outdated Show resolved Hide resolved
dipu/QuickStart.md Outdated Show resolved Hide resolved
dipu/torch_dipu/csrc_dipu/aten/RegisterDIPU.hpp Outdated Show resolved Hide resolved
#define DIOPI_ATEN_FUNC(opname, diopiFunc, wapperFunc) \
#define addAutoCompare(wrapperFunc) wrapperFunc##_autocompare
#define DIOPI_ATEN_FUNC(opname, diopiFunc, wrapperFunc) \
do { \
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
Collaborator Author

Choose a reason for hiding this comment

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

我把嵌套的if条件拆开了

dipu/torch_dipu/csrc_dipu/aten/RegisterDIPU.hpp Outdated Show resolved Hide resolved
} \
} while (false);

#define DIOPI_ATEN_FUNC_DISABLE_AUTOCOMPARE(opname, diopiFunc, wrapperFunc) \
do { \
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

// "SPECIFIED_AUTOCOMPARE_OPS_LIST"; specified_autocompare_config_name =
// ".specified_autocompare_op_list.config"

namespace dipu {
Copy link
Collaborator

Choose a reason for hiding this comment

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

namespace 太浅了

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

我把正则匹配相关的函数和变量放到了namespace dipu::opRegexMatch下面

Copy link
Collaborator

Choose a reason for hiding this comment

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

namespace 小写下划线吧

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

dipu/torch_dipu/csrc_dipu/aten/ops/OpRegexMatch.cpp Outdated Show resolved Hide resolved
dipu/torch_dipu/csrc_dipu/aten/ops/OpRegexMatch.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

考虑到用户对正则的不熟悉,是否有必要改成 wildcard?

Copy link
Collaborator Author

@NeosZhang NeosZhang Apr 22, 2024

Choose a reason for hiding this comment

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

多数场景下,算子匹配并不会用到很复杂的正则的语法,在readme里面多给几个常用的场景的示例就可以了。
但如果换成wildcard,灵活性会差很多,可能不能满足一些极端场景的需求

dipu/torch_dipu/csrc_dipu/aten/ops/OpRegexMatch.cpp Outdated Show resolved Hide resolved
@lljbash
Copy link
Collaborator

lljbash commented Apr 22, 2024

copy 是否已支持 autocompare,是不是要把 disable 去掉

@NeosZhang NeosZhang added DIPU DIPU related refactor just better coding labels Apr 22, 2024
@NeosZhang NeosZhang requested a review from lljbash April 23, 2024 08:08
Copy link
Collaborator

@lljbash lljbash left a comment

Choose a reason for hiding this comment

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

之前 comment 的逻辑简化依然应该做,if else 那堆现在特别难读

// "SPECIFIED_AUTOCOMPARE_OPS_LIST"; specified_autocompare_config_name =
// ".specified_autocompare_op_list.config"

namespace dipu {
Copy link
Collaborator

Choose a reason for hiding this comment

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

namespace 小写下划线吧

return list;
}

bool whetherOpMatch(const char* opname,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
bool whetherOpMatch(const char* opname,
bool isOpMatch(const char* opname,

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
Collaborator Author

Choose a reason for hiding this comment

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

done

Comment on lines 16 to 21
extern const char* const fallback_env_name;
extern const char* const fallback_config_name;
extern const std::vector<std::regex> fallbackMatchers;

extern const char* const specified_autocompare_env_name;
extern const char* const specified_autocompare_config_name;
Copy link
Collaborator

Choose a reason for hiding this comment

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

const char* 不用 extern,用 constexpr

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Comment on lines 223 to 227
- 可以通过设置环境变量`SPECIFIED_AUTOCOMPARE_OPS_LIST=‘.*’`,开启全局的精度对比,这种情况下所有调用的算子都会进行精度对比。

```shell
# 开启全局的算子精度自动对比功能
export SPECIFIED_AUTOCOMPARE_OPS_LIST=‘.*’
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- 可以通过设置环境变量`SPECIFIED_AUTOCOMPARE_OPS_LIST=‘.*’`,开启全局的精度对比,这种情况下所有调用的算子都会进行精度对比。
```shell
# 开启全局的算子精度自动对比功能
export SPECIFIED_AUTOCOMPARE_OPS_LIST=‘.*
- 可以通过设置环境变量`SPECIFIED_AUTOCOMPARE_OPS_LIST='.*'`,开启全局的精度对比,这种情况下所有调用的算子都会进行精度对比。
```shell
# 开启全局的算子精度自动对比功能
export SPECIFIED_AUTOCOMPARE_OPS_LIST='.*'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done


```shell
# 指定匹配add.*?的算子进行自动精度对比
export DIPU_AUTOCOMPARE_OPS_LIST=add.*?
Copy link
Collaborator

Choose a reason for hiding this comment

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

我还是很奇怪,add.*? 是个啥匹配

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

image add.*?非贪婪匹配所有add开头的,匹配结果是add 比如"add.out, add.scalar", 匹配结果就是“add”, “add”,只要匹配结果不为空,isOpMatch都返回True

Copy link
Collaborator

Choose a reason for hiding this comment

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

问号是对匹配结果造成影响是吧,这么看来这这里是多余的?

Comment on lines +64 to +76
constexpr const char* fallback_env_name = "DIPU_FORCE_FALLBACK_OPS_LIST";
constexpr const char* fallback_config_name =
".dipu_force_fallback_op_list.config";
const std::vector<std::regex> fallbackMatchers =
dipu::op_regex_match::loadMatcher(fallback_env_name, fallback_config_name);

constexpr const char* specified_autocompare_env_name =
"DIPU_AUTOCOMPARE_OPS_LIST";
constexpr const char* specified_autocompare_config_name =
".specified_autocompare_op_list.config";
const std::vector<std::regex> autocompareMatchers =
dipu::op_regex_match::loadMatcher(specified_autocompare_env_name,
specified_autocompare_config_name);
Copy link
Collaborator

Choose a reason for hiding this comment

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

我建议按常量命名规范 k+驼峰

@lljbash
Copy link
Collaborator

lljbash commented Apr 25, 2024

FYI: clang-format 根目录有个脚本可以用

else { \
m.impl(opname, TORCH_FN(wrapper_func)); \
} \
#define CONCAT_NAME(a, b) a##b
Copy link
Collaborator

@lljbash lljbash Apr 25, 2024

Choose a reason for hiding this comment

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

我记得这样是不会展开 a 和 b 的,还得加一层。
不过我们某个头文件里可能已经实现这个基础设施了,没有的话也应该加在那里,因为这个很基础
或者用 C10_CONCATENATE
而且宏绝对不能这么起名字,太容易撞了

@NeosZhang NeosZhang marked this pull request as draft April 26, 2024 03:44
xuq7410 pushed a commit to xuq7410/deeplink.framework that referenced this pull request May 23, 2024
* fix cat bug

* use cast in op-plugin and optimize cat

* skip cat double test case

* enhance empty check

* remove OpCommandImpls relate

* use diopiCopyInp and diopiDtypeCast

* fallback cast to cpu

* skip some test case: contiguous to no contiguous

* fix copy and cast bug

* support stride_copy_support double

* enable some test case

* skip double test case in copy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DIPU DIPU related refactor just better coding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants