-
Notifications
You must be signed in to change notification settings - Fork 6.7k
Test large vector mean operator and fix a few bugs #16079
Conversation
|
|
||
| index_t s = 1; | ||
| #pragma unroll | ||
| for (int i = ndim-1, j = mdim, s = 1; i >= 0; --i) { |
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.
nice catch ! :)
tests/nightly/test_large_vector.py
Outdated
|
|
||
|
|
||
| def test_mean(): | ||
| a = nd.arange(-LARGE_X / 2, LARGE_X / 2 + 1, dtype=np.int64) |
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.
nit: even though LARGE_X is divisible by 2 but can we use integer division here LARGE_X//2
ChaiBapchya
left a comment
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!
| a = create_vector(LARGE_X) | ||
| res = nd.clip(a, a_min=100, a_max=1000) | ||
| assert np.sum(res[-1].asnumpy() == 1000) == 1 | ||
| assert res[-1] == 1000 |
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.
good ... previous one was bad
|
Nice work ! |
|
@access2rohit My test test_mean runs through. However, test_sequence_last seems to run out of memory on an instance with 480G memory. test_large_vector.test_slice ... ok Segmentation fault: 11 Stack trace: |
|
@wkcn Can you help to review and merge this for me? |
wkcn
left a comment
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.
Thank you for the update!
In the unittest test_ndarray_random_randint,
it will be specific to use 2 ** 32 rather than 4294967296.
wkcn
left a comment
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. I will merge it after the CI pass.
* add test_mean and fix a few potential bugs * address reviewer commnet * fix and refactor test * fix topk test * address reviewer comment
* add test_mean and fix a few potential bugs * address reviewer commnet * fix and refactor test * fix topk test * address reviewer comment
* add test_mean and fix a few potential bugs * address reviewer commnet * fix and refactor test * fix topk test * address reviewer comment
* add test_mean and fix a few potential bugs * address reviewer commnet * fix and refactor test * fix topk test * address reviewer comment
* add test_mean and fix a few potential bugs * address reviewer commnet * fix and refactor test * fix topk test * address reviewer comment
Description