Merge doFFT_Transposed_Elegant and doFFT_Transposed_Performant

[ronawho]

The first 2 commits remove the algorithmic differences between elegant and performant. The last commit merges the functions since most of the code is identical.

[ronawho]

@npadmana easiest to review ignoring whitespace.

My thought was that performance vs elegant should track Chapel issues and that algorithmic differences aren't worth tracking since we could never expect the worse algorithm to compete.

[npadmana]

I had a somewhat different motivation when writing elegant -- it quite closely resembles a naive implementation of the pencil-and-paper description of a multidimensional FFT. And so I thought it was good starting point to show.

One can then go from this elegant version and optimize. What is really nice about the updates you made was that you can cleanly separate out that logic from the Chapel code -- so one might imagine flipping between the two and seeing how small the differences are.

However, I agree that from an optimization point of view, it's worth keeping the algorithms the same, and just switching between different communication modes.

So maybe I'm just arguing to keep the original elegant code around, even if commented out.

[ronawho]

I had a somewhat different motivation when writing elegant -- it quite closely resembles a naive implementation of the pencil-and-paper description of a multidimensional FFT. And so I thought it was good starting point to show.

One can then go from this elegant version and optimize. What is really nice about the updates you made was that you can cleanly separate out that logic from the Chapel code -- so one might imagine flipping between the two and seeing how small the differences are.

Ah interesting. My thought had been that we had hidden most of the optimization logic, so the elegant and performant version really didn't look that different aside from comm primitives and so the performant chapel version looked like the pencil-and-paper version.

Hmm, I'm not sure what to do with this then. From my perspective of optimizing Chapel so we don't need the comm primitives, I definitely want to eliminate algorithmic differences, but I can capture a snapshot for those explorations.

I'm happy to close this and keep the elegant version as-is if you want. Left to my own devices what I might do is rename the elegant version naive and have the performant version still contain the usePrimitiveComms switch.

[npadmana]

Actually, the naive version is really only there for PAW and if we write this up as a paper. And it's useful for me as a mnemonic on how one might overlap communication with computation (without resorting to explicit task creation). But I can always look at the version of the code at a given tag.

So just go ahead and blow away the naive branch.

[ronawho]

https://github.com/npadmana/DistributedFFT/compare/master...ronawho:split-naive if you want to see what keeping the naive version might look like. I'm happy to push with either branch (and if we go with naive version, that leaves it up to you to decide if you want to blow away the naive version.)

[npadmana]

Let's do the split-naive. Thanks!

[ronawho]

Closing in favor of https://github.com/npadmana/DistributedFFT/pull/44