-
Notifications
You must be signed in to change notification settings - Fork 29
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
base: main
Are you sure you want to change the base?
Zq/add specified autocompare #785
Conversation
#define DIOPI_ATEN_FUNC(opname, diopiFunc, wapperFunc) \ | ||
#define addAutoCompare(wrapperFunc) wrapperFunc##_autocompare | ||
#define DIOPI_ATEN_FUNC(opname, diopiFunc, wrapperFunc) \ | ||
do { \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
嵌套太多层了,这里的逻辑可以简化很多
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我把嵌套的if条件拆开了
} \ | ||
} while (false); | ||
|
||
#define DIOPI_ATEN_FUNC_DISABLE_AUTOCOMPARE(opname, diopiFunc, wrapperFunc) \ | ||
do { \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
namespace 太浅了
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我把正则匹配相关的函数和变量放到了namespace dipu::opRegexMatch下面
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
namespace 小写下划线吧
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
考虑到用户对正则的不熟悉,是否有必要改成 wildcard?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
多数场景下,算子匹配并不会用到很复杂的正则的语法,在readme里面多给几个常用的场景的示例就可以了。
但如果换成wildcard,灵活性会差很多,可能不能满足一些极端场景的需求
copy 是否已支持 autocompare,是不是要把 disable 去掉 |
Co-authored-by: Lingjie <[email protected]>
There was a problem hiding this 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 { |
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bool whetherOpMatch(const char* opname, | |
bool isOpMatch(const char* opname, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里忘了删吧
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const char* 不用 extern,用 constexpr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
dipu/QuickStart.md
Outdated
- 可以通过设置环境变量`SPECIFIED_AUTOCOMPARE_OPS_LIST=‘.*’`,开启全局的精度对比,这种情况下所有调用的算子都会进行精度对比。 | ||
|
||
```shell | ||
# 开启全局的算子精度自动对比功能 | ||
export SPECIFIED_AUTOCOMPARE_OPS_LIST=‘.*’ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 可以通过设置环境变量`SPECIFIED_AUTOCOMPARE_OPS_LIST=‘.*’`,开启全局的精度对比,这种情况下所有调用的算子都会进行精度对比。 | |
```shell | |
# 开启全局的算子精度自动对比功能 | |
export SPECIFIED_AUTOCOMPARE_OPS_LIST=‘.*’ | |
- 可以通过设置环境变量`SPECIFIED_AUTOCOMPARE_OPS_LIST='.*'`,开启全局的精度对比,这种情况下所有调用的算子都会进行精度对比。 | |
```shell | |
# 开启全局的算子精度自动对比功能 | |
export SPECIFIED_AUTOCOMPARE_OPS_LIST='.*' |
There was a problem hiding this comment.
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.*? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我还是很奇怪,add.*?
是个啥匹配
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
问号是对匹配结果造成影响是吧,这么看来这这里是多余的?
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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我建议按常量命名规范 k+驼峰
FYI: clang-format 根目录有个脚本可以用 |
else { \ | ||
m.impl(opname, TORCH_FN(wrapper_func)); \ | ||
} \ | ||
#define CONCAT_NAME(a, b) a##b |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我记得这样是不会展开 a 和 b 的,还得加一层。
不过我们某个头文件里可能已经实现这个基础设施了,没有的话也应该加在那里,因为这个很基础
或者用 C10_CONCATENATE
而且宏绝对不能这么起名字,太容易撞了
* 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
重构autocompare,autocompare功能的开启或关闭不再依赖编译,和DIPU_AUTOCOMPARE_OPS_LIST来设置autocompare功能,可以指定算子进行autocompare。