-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
fill_diagonal op fix border cross caused by offset #36212
fill_diagonal op fix border cross caused by offset #36212
Conversation
Thanks for your contribution! |
if (i % out_dims[1] + offset >= 0 && | ||
i % out_dims[1] + offset < out_dims[1]) { |
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.
suggest adding a annotation to explain this if statement
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
if ((idx * strides) % dims + offset < dims && | ||
(idx * strides) % dims + offset >= 0) { |
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.
suggest adding a annotation to explain this if statement
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
and it would be better to add a unittest to cover this case |
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
PR types
Bug fixes
PR changes
OPs
Describe
fill_diagonal op fix border cross caused by offset
原来的方式offset非0时修改值会出现跨行的问题,
本次修复添加行位置越界判断,避免了这类问题。
修复前:
x.fill_diagonal_(1,offset=2)
修改了(0,2)、(2,0)两个位置,其中(2,0)由(1,1)右移2(offset值)导致。
修复后:
x.fill_diagonal_(4,offset=2)
只修改了(0,2)位置