FFT refactoring

[magnatelee]

This PR makes the following changes:

1. refactoring on FFT type class (FFT_C2C and others) to reduce repetitions
2. refactoring on FFT tests and adding more test cases with reshaped inputs
3. removing the exact mapping constraint for FFT inputs (which wasn't doing exactly what it was meant to do) and instead making copies inside FFT tasks whenever necessary
4. removing unnecessary temporary allocations for FFT outputs

[mferreravila]

Why are we losing the larger test cases?

[magnatelee]

They are removed because they don't increase test coverage. All tests in the testing mode exercise the deferred path, and there's a separate mode for testing the eager path. Tests themselves don't need to be aware of what testing mode they run in. As far as I know, the FFT task is insensitive to the input size, so passing two different input sizes doesn't seem to add much other than testing cuFFT in different ways, which I believe should be done by the cuFFT test suite.

[mferreravila]

I agree that cuFFT runs its own test cases for different sizes. However, numerical errors due to incorrect input parameters tend to become more evident with larger test cases. I question if the resources you are saving by not running these cases are worth it.

[mferreravila]

In any case, I'll wait for an appropriate channel to discuss feedback.

[magnatelee]

However, numerical errors due to incorrect input parameters tend to become more evident with larger test cases

if there's a source of numerical errors other than cuFFT itself, I guess we could add back some of the tests with big inputs, but even then, I'd rather isolate them from functionality tests that exercise various parameters. And I don't think there's much of computation that could either worsen or mask the numerical errors from cuFFT.

Also, "numerical errors due to incorrect input parameters" don't make sense to me. If the input parameters are "incorrect", those should be caught rather than being passed to the rest of computation. (I suspect you were trying to say "suboptimal input parameters"?) If for some reason you want to do some additional numerical testing that the existing cuFFT test suite doesn't do, I guess it's fine to throw them here, but I feel those should rather be done to cuFFT itself...

[manopapad]

I assume this change was related to this comment

removing the exact mapping constraint for FFT inputs (which wasn't doing exactly what it was meant to do)

Can you expand on that? What was wrong with the way it was done before?

[magnatelee]

The dimension ordering doesn't take the store transforms into account and is always absolute with respect to the original coordinate space. Because of this, the exact constraint alone cannot guarantee the exact instance in C order that cuFFT expects, if the store was transposed. Currently, the only way to check this is to make an affine accessor with the affine transform and call is_dense_row_major. Once we start supporting relative dimension ordering, I guess we could put back the exact constraint and revert the change in the FFT task, but even then the exact constraint is unnecessary for certain types of FFTs we are doing; for example, forward and backward FFTs over axes make copies of the input internally, so the exact constraint only makes a copy that would be thrown away immediately: https://github.com/nv-legate/cunumeric/blob/branch-22.05/src/cunumeric/fft/fft.cu#L262

[magnatelee]

FYI, the exact constraint we would potentially want is something like this:

// scalars[2] being true means we're doing FFT over axes, which makes a copy inside the task
if (!task.scalars()[2].value<bool>()) mappings.back().policy.exact = true;