-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Disable pool&conv_transpose&quantize caching #36695
Conversation
Thanks for your contribution! |
@wozna Please take a look at changes in quantize. |
@tsocha Please review |
for (int j = 0; j < o; ++j) { | ||
int in_offset = j * h * w + i * o * h * w; | ||
int out_offset = j * c * h * w + i * h * w; | ||
std::memcpy(&(reordered_filter_data.get())[out_offset], |
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.
I know that might not be your code, but shouldn't that reorder be done using oneDNN?
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.
:) @Silv3S is working on that.
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.
LGTM
@chenwhql Could you please approve so that PR-CI-APPROVAL pass? PADDLE_ENFORCE is correct but due only part of it is visible in diff, corresponding checker cannot see full statement and report error. |
@baoachun @Superjomn This PR is part of the improving cache mechanism. CI reported PADDLE_ENFORCE error but actually it is correct but due only part of it is visible in diff, corresponding checker cannot see full statement and report error. |
platform::errors::InvalidArgument( | ||
"Got wrong formats for Filter tensor.")); | ||
"Got wrong format for Bias tensor.")); |
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.
Hi, @baoachun
这里,错误信息长度符合要求,要求至少20个字符长度,这里错误信息有30个字符左右长度。这里的问题是PADDLE_ENFORCE 机制不认未更改的行,这里 95行 platform::errors::InvalidArgument(
已经写了,只是这个PR没有更改,所以就没检测到,图片里InvalidArgument代码就没有。但是其实代码中是有的,是PADLLE_ENFORCE机制没检测到。
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.
这个PADDLE_ENFORCE机制以后会考虑优化吗,以后可能也经常有类似问题。
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.
这里是字符串正则匹配检测的,先grep找出修改了的行,然后正则匹配做合规检查,如果全文件正则检查,可以避免这个问题,但是可能会检出和PR不相关的一些问题,不太友好。。。目前这一点没有想到好的办法处理,先找我approve吧
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.
LGTM
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.
LGTM
Sorry to inform you that b27d1b8's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually. |
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.
LGTM for PADDLE_ENFORCE
@jczaja This PR is approved by Baidu. But could you please watch PR-CI-iScan-C to pass ? Cause only this one keep failing. |
@baoachun Please help with failing CI (it did pass few reruns ago). Error is: "git code download failed ". I have try couple of reruns but without success. |
My colleague is solving the problem, please wait. |
Hi @jczaja, my colleague said that you may need to pull the latest code of the develop branch. |
- pool2d partially stripped of caching
8567968
b27d1b8
to
8567968
Compare
@baoachun I have rebased this PR but still PR-CI-iScan-C is failing (I have done one rerurn). Please help |
PR types
Bug fixes
PR changes
OPs
Describe
PaddlePaddle is caching oneDNN objects and this PR is to disable this for pool2d and conv_transpose. Reason is that oneDNN is caching some of its objects itself