Refined 'tests/array_split.py' w/ more essential input shapes #178
Conversation
tests/array_split.py
Outdated
| # divisible integer | ||
| input_arr.append(even_div) | ||
| # indivisble integer | ||
| if even_div != uneven_div and uneven_div is not None: |
There was a problem hiding this comment.
the first condition seems very confusing. I believe even_div and uneven_div meant to be different, but why are you checking if they are different?
There was a problem hiding this comment.
It can both 1 if a.shape[axis] is prime.
There was a problem hiding this comment.
ok then is there reason that you don't test with even_div == a.shape[axis] in that case? I feel this is another unnecessary complication in this test that is just confusing.
tests/array_split.py
Outdated
| even_div = div | ||
| else: | ||
| uneven_div = div | ||
| if even_div is not None and uneven_div is not None: |
There was a problem hiding this comment.
I don't think I'm a big fan of this condition, as the values that would be chosen for even_div and uneven_div are somewhat unpredictable. I feel we're trying to be too smart here, as we won't really run this test with non-default test parameters.
There was a problem hiding this comment.
I see. This tests take any size of matrix so the dimension could be prime or not, which I tried to handle that.
What do you mean by 'non-defualt' test parameters?
There was a problem hiding this comment.
I mean, practically no one will try to run this unit test with the size overriden by the --dim_size flag. I'm saying the test is overly complicated for cases that would happen very rarely.
tests/array_split.py
Outdated
| c = getattr(num, routine)(a, input_opt, axis) | ||
| is_equal = True | ||
| err_arr = [b, c] | ||
| for routine, input_opt in itertools.product( |
There was a problem hiding this comment.
again, what's the point of doing product if this test only tests array_split?
There was a problem hiding this comment.
It used to have split w/ error checking codes to see if both cunumeric and numpy produces the same error for non-viable options. I'll delete this
tests/array_split.py
Outdated
| err_arr = [b, c] | ||
| else: | ||
| for each in zip(b, c): | ||
| sub_set = list(each) |
There was a problem hiding this comment.
why do you need to create a fresh list here? isn't each already a tuple that you can access with index expression?
tests/array_split.py
Outdated
| print_msg = f"np.{routine}({a.shape}, {input_opt}" f", {axis})" | ||
| # Check if both impls produce the error | ||
| # for non-viable options | ||
| b = getattr(np, routine)(a, input_opt, axis) |
There was a problem hiding this comment.
again, why can't you just call np.array_split here?
|
Looks good to me. |
Refined this test w/ reduced input shapes.
Instead, more essential test cases are included w/ empty, singleton arrays