Pass zShift to ShiftedMetric constructor

[ZedThree]

Moved the logic for getting zShift from the grid file, etc., into Mesh::setParallelTransform. ShiftedMetric ctor now explicitly takes zShift, which technically breaks backwards compatibility, but if you're calling the ctor yourself, you probably know what you're doing?
We could keep backwards compatibility by moving all the ctor logic into a separate init method or something (can't just call the other ctor as we zShift first)

This allows greater separation between ShiftedMetric and Mesh which should make it easier to test ShiftedMetric

[johnomotani]

I've been wondering if zShift should be moved from ShiftedMetric to Coordinates to make getting staggered-grid versions easier? When I was writing ShiftToFieldAligned in #1177 (before Coordinates at different locations) I had to have 3 different zShift variables to store the interpolated versions as well as the CELL_CENTRE one.

This would be an alternative way to keep backward compatibility, since you wouldn't need to pass zShift to the constructor at all, just fetch it from localmesh->getCoordinates()->zShift.

[ZedThree]

That is a bit painful. I'm not sure about putting zShift into coordinates though, as it's really a ShiftedMetric thing.

What if we moved all the internal variables into a struct, ShiftedInternals, and then had a map<CELL_LOC, ShiftedInternals>. The caching that's done in the ctor currently could go in a private getOrMakeCache(CELL_LOC) method that's called for f.getLocation() in calcYUpDown?

• "Just works" for any location
• First call is slow, subsequent calls faster
• Better names to be decided

@d7919 @johnomotani does that sound workable?

[johnomotani]

@ZedThree I think of zShift as some information about the coordinate system (e.g. it gives the toroidal position of points with paralleltransform=identity) so maybe belongs in Coordinates. Also, if/when I add ShiftToFieldAligned (assuming it stays as a separate class) then it would need some copied logic for loading zShift (and the logic is not totally trivial if we add/subtract ShiftAngle in order to get avoid one set of FFTs in the branch-cut guard cells see #1294).

I think the struct+map thing sounds good, but is not enough to make ShiftedMetric useful for y-staggered grids: I'd leave ShiftedMetric as is*, and leave it to ShiftToFieldAligned to handle the staggered grid cases (I'll use the design you suggest above when I get around to updating #1177).

--
*i.e. not compatible with staggering, we could allow for example staggering in the x-direction by having a staggered zShift, etc. but it doesn't seem worth implementing that just for the sake of it

[johnomotani]

What if we moved all the internal variables into a struct, ShiftedInternals, and then had a map<CELL_LOC, ShiftedInternals>.

Another option might be to make ParallelTransforms have a location and keep a map<CELL_LOC, ParallelTransform> in the Mesh, like we now do for Coordinates. We could rename Mesh::addCoordinates to Mesh::addLocation and initialize both Coordinates and ParallelTransform in the same method.

[ZedThree]

We really need to sit down and properly design how all these systems should interact, and what belongs to what. @d7919 has suggested that the whole parallel transform needs to be moved into Coordinates, which I agree with. This might then go at least some way to taking care of the staggered grids problem.

---

Closing this as #1413 achieves the same thing, but is backwards compatible