[TEST][FLAKY] topi/tests/python/test_topi_sort.py::test_argsort #4891
Conversation
|
in order to avoid ties, we might need to take an array whose element is sufficient apart, e.g. range(n) |
|
Hi @tqchen I just update the PR. I am not sure if I understand the suggestion correctly, I would appreciate that you can describe it further. Sorry for any inconvenience. many thanks, |
|
OK, sorry for not being clear, the root problem is potential ties in the value generated by np.random(two values too close to each other). So one way to avoid that is only generate random value that we know won't have ties. For example np.shuffle(range(n))Then we can use this as input data to test argsort |
|
Hi @tqchen Thanks for the prompt reply and explanation. I shuffle index and get data from the shuffled index. |
topi/tests/python/test_topi_sort.py
Outdated
| @@ -27,6 +27,12 @@ def verify_argsort(axis, is_ascend): | |||
| data_dtype = "float32" | |||
| data = tvm.placeholder(dshape, name="data", dtype=data_dtype) | |||
| np_data = np.random.uniform(size=dshape).astype(data_dtype) | |||
There was a problem hiding this comment.
Do you mean the output nd.array of numpy.random.uniform?
There was a problem hiding this comment.
np data is provided in the next few lines, this line is not needed
There was a problem hiding this comment.
I think np_data is needed by the following lines, like L32 and L37, but please correct me if I’m wrong on that
There was a problem hiding this comment.
Ah, sorry I was a bit confused here. The shuffling here still does not address the problem of ties(two values too close to each other in random.uniform).
We should instead change this line to directly create arange then broadcast to the shape, then we shuffle each of the broadcasted place.
Then the problem will be solved because arange won't have ties
There was a problem hiding this comment.
Apology for the misunderstanding. I update that part you mentioned and the test results as shown below:
>>> perm = np.arange(dshape[0] * dshape[1], dtype=data_dtype)
>>> np.random.shuffle(perm)
>>> np_data = perm.reshape(dshape)
>>> np_data
array([[22., 23., 10., 18., 12.],
[16., 20., 6., 21., 1.],
[24., 7., 4., 15., 13.],
[14., 5., 8., 19., 11.],
[ 9., 3., 0., 2., 17.]], dtype=float32)
topi/tests/python/test_topi_sort.py
Outdated
| @@ -27,6 +27,12 @@ def verify_argsort(axis, is_ascend): | |||
| data_dtype = "float32" | |||
| data = tvm.placeholder(dshape, name="data", dtype=data_dtype) | |||
| np_data = np.random.uniform(size=dshape).astype(data_dtype) | |||
There was a problem hiding this comment.
Ah, sorry I was a bit confused here. The shuffling here still does not address the problem of ties(two values too close to each other in random.uniform).
We should instead change this line to directly create arange then broadcast to the shape, then we shuffle each of the broadcasted place.
Then the problem will be solved because arange won't have ties
tqchen
added
status: accepted
and removed
status: need update
|
Hi @tqchen Thanks for your patience and I did learn a lot from this task. :) |
…he#4891) * [TEST][FLAKY] topi/tests/python/test_topi_sort.py::test_argsort * upadate test function of argsort like topk * Shuffle index and get data from shuffled index * Replace the random.uniform with np.arange
…he#4891) * [TEST][FLAKY] topi/tests/python/test_topi_sort.py::test_argsort * upadate test function of argsort like topk * Shuffle index and get data from shuffled index * Replace the random.uniform with np.arange
…he#4891) * [TEST][FLAKY] topi/tests/python/test_topi_sort.py::test_argsort * upadate test function of argsort like topk * Shuffle index and get data from shuffled index * Replace the random.uniform with np.arange
Hi @tqchen ,
Following issue #4860 , I sort the array and then shuffle it, I would appreciate if you can give some suggestions/review for this PR, thank you.