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

Fix inconsistency between index_fill and index_fill_ #59863

Merged
merged 3 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion python/paddle/tensor/manipulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -5887,8 +5887,9 @@ def _index_fill_impl(x, index, axis, value, inplace):
perm[axis] = 0

if inplace:
paddle.transpose(x, perm)
Copy link
Contributor

Choose a reason for hiding this comment

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

目前有inplace版本paddle.transpose_

此外,不建议在该API中使用索引语法实现赋值,会带来解析索引的耗时,此前使用transpose + index_put的逻辑,本质上是已经根据该场景解析过后决定了要调用哪些API

Copy link
Contributor Author

@Patrick-Star125 Patrick-Star125 Dec 11, 2023

Choose a reason for hiding this comment

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

1.目前paddle文档上没有paddle.transpose_
2.使用paddle.transpose_实现会有inplace_version跳变问题,inplace_version貌似在python端只能增不能减,所以我更改了一部分version相关测试

paddle.transpose_(x, perm)
paddle.index_put_(x, (index,), value)
paddle.transpose_(x, perm)
return x
else:
out = paddle.transpose(x, perm)
Expand Down
34 changes: 33 additions & 1 deletion test/legacy_test/test_inplace.py
Original file line number Diff line number Diff line change
Expand Up @@ -1621,7 +1621,7 @@ class TestDygraphInplaceIndexFill(TestDygraphInplace):
def init_data(self):
self.input_var_numpy = np.random.random((20, 40))
self.dtype = "float32"
self.axis = 0
self.axis = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

这个修改是否是必要的呢,目前axis=0是否表现正常?

Copy link
Contributor Author

@Patrick-Star125 Patrick-Star125 Dec 11, 2023

Choose a reason for hiding this comment

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

axis=0是正常的,axis=1覆盖情况更广,之前就是因为axis=0导致没有测试到这个API的inplace版本有不一致问题

self.index = paddle.to_tensor([0, 2])
self.value = -1

Expand All @@ -1631,6 +1631,38 @@ def inplace_api_processing(self, var):
def non_inplace_api_processing(self, var):
return paddle.index_fill(var, self.index, self.axis, self.value)

def test_forward_version(self):
with paddle.base.dygraph.guard():
var = paddle.to_tensor(self.input_var_numpy).astype(self.dtype)
self.assertEqual(var.inplace_version, 0)

inplace_var = self.inplace_api_processing(var)
self.assertEqual(var.inplace_version, 3)

inplace_var[0] = 2
self.assertEqual(var.inplace_version, 4)

inplace_var = self.inplace_api_processing(inplace_var)
self.assertEqual(var.inplace_version, 7)

def test_backward_error(self):
with paddle.base.dygraph.guard():
var_a = paddle.to_tensor(self.input_var_numpy).astype(self.dtype)
var_a.stop_gradient = False

var_b = var_a**2

var_c = var_b**2
self.inplace_api_processing(var_b)
var_c = paddle.cast(var_c, "float32")

loss = paddle.nn.functional.relu(var_c)
with self.assertRaisesRegex(
RuntimeError,
f"received tensor_version:{3} != wrapper_version_snapshot:{0}",
):
loss.backward()


if __name__ == '__main__':
unittest.main()