Chapel: UnitTest assumes 1-based arrays (and possibly 1D arrays?)

[bradcray]

While working on some test updates, I noticed that the current UnitTest framework seems to assume that arrays use 1-based indexing (and it appears that it may assume that arrays are one-dimensional as well?). As an example, consider the following:

use UnitTest;

var test = new Test();
var x1: [0..2] int = [1,5,7];
var y1: [0..2] int  = [1,7,6];
test.assertEqual(x1, y1);

[bradcray]

Attention @krishnadey30, @ben-albrecht

[bradcray]

Thinking about this a few minutes more, it seems as though the best way to write an array equality check would be to leverage Chapel more, for example, once you'd asserted what you wanted about their types/sizes/whatever, you could just do && reduce (arr1 == arr2) which would leverage parallelism, scale better for distributed arrays, be index neutral, and avoid reinventing the wheel.

[lydia-duncan]

and it appears that it may assume that arrays are one-dimensional as well?

It looks like if the rank != 1, we rely on array.equals instead of the sequence traversal, so we should do the appropriate thing for multidimensional arrays?

[bradcray]

Huh... I wonder why rank == 1 was special-cased...?

[ben-albrecht]

I wonder why rank == 1 was special-cased...?

The library provides error messages that include the specific elements that did not match. This was only implemented for the 1D case, with an outstanding TODO to extend to N-dimensional case.

I have a branch that fixes this, which uses the && reduce (arr1 == arr2) suggestion from above. This slightly changes the error message capabilities, because we no longer report the exact index - but I think that's OK in the short term.

I will open a PR for this as is and open a separate issue to explore how we want to generate a helpful error messages for the general N-dimensional array case.

[bradcray]

Some ideas about improving the errors:

If time was no object, when an error occurs, you could run a second forall across the (arr1==arr2) comparison, having the first processor to find such a case take a lock, print the message, and short-circuit as much of the rest of the computation as possible (having other processors do the same if/when they notice the message). I.e., write a home-grown eureka.

Or store the (arr1 == arr2) expression into an array of bools (always? or just when an error has been identified?)

Or you could make it a serial loop, which is no worse than what's on master today, but I worry about the cost that could incur for a large distributed array (but maybe it's OK to assume that unit tests don't use very large arrays? I'm not sure...).

[ben-albrecht]

Thanks for the ideas @bradcray.

I'm less sure about how to format such an error message though. Should we print the full arrays, just the mismatched values, or just the mismatched indices? When do we summarize the diffs vs. dump all the diffs?

[krishnadey30]

I'm less sure about how to format such an error message though. Should we print the full arrays, just the mismatched values, or just the mismatched indices? When do we summarize the diffs vs. dump all the diffs?

I think just printing the first mismatch will be better as we don't know that others are mismatched due to the first one. Eg. [1,2,3]& [1,4,2,3]

Also, I haven't implemented the assertions for the list type. I think we should start working on those

[ben-albrecht]

@krishnadey30 - one challenge is the scalability of printing this information, e.g. printing a 1Mx1M array is not going to be useful to the user. I've opened https://github.com/chapel-lang/chapel/issues/14330 to explore this issue further.

[ben-albrecht]

I think just printing the first mismatch will be better as we don't know that others are mismatched due to the first one. Eg. [1,2,3]& [1,4,2,3]

For size mismatch errors like this, I don't think we need to inspect the values at all.