Location of yup/ydown doesn't respect parent location

[d7919]

In splitYupYdown the yup and ydown fields are created but their location is not set, so they exist at the default location. Presumably these should really be set to the same location as the parent field?

Should calling setLocation on the parent also call setLocation on the yup/ydown fields as well?

[johnomotani]

zShift depends on the location, so in principle yup/ydown need to be calculated with the location set correctly (although I don't think this is implemented at the moment; I was working on it at one point, but need to go back after the Coordinates location updates). So setLocation should probably invalidate yup/ydown fields rather than change their location.

[d7919]

zShift depends on the location, so in principle yup/ydown need to be calculated with the location set correctly (although I don't think this is implemented at the moment; I was working on it at one point, but need to go back after the Coordinates location updates). So setLocation should probably invalidate yup/ydown fields rather than change their location.

Yes I was wondering about this -- setLocation doesn't actually change the values in the field either so I viewed it as -- the user/code now says I've setup this field correctly to be at the location I'm telling you (and hence the yup/ydown location should also be changed) rather than asking for the location of the field to be somewhere else. From the sounds of it the "correct" thing is to throw if we try to calcYupYdown on anything not at cell centre for now?

[d7919]

(but this is probably only needed for shifted).

[johnomotani]

From the sounds of it the "correct" thing is to throw if we try to calcYupYdown on anything not at cell centre for now?

I think this is the correct thing to do in ShiftedMetric and FCI, which don't deal with staggered grids (and probably never will). ParallelTransformIdentity is fine, because ParallelTransformIdentity::calcYUpDown just does f.mergeYupYdown();

[d7919]

Could this also be an issue for ddt(field)?

[d7919]

@johnomotani and @ZedThree I'm looking at the index derivative code at the moment and in particular at the y derivatives where we shift to/from field aligned. I notice that to/from field aligned doesn't (in general) set the output field's location, which suggests to me that the staggered derivatives using the field aligned fields are probably incorrect. Do you agree and if so should we just carry on doing what we're doing, should we set the location (I think the earlier discussion here suggests this would not be consistent with the location of the shifted metric etc.) or should we abort?

[johnomotani]

@d7919 I think to/fromFieldAligned should check and set the location of their results. The ShiftedMetric implementation should probably give an error if it's input is not CELL_CENTRE. When ShiftToFieldAligned from #1177 is updated/added (I'm working on it slowly in https://github.com/boutproject/BOUT-dev/tree/new_shifted_metric-rebasing), it will be able to handle inputs at any location and return a correct field-aligned output.

For now I suggest I update #1176 with checks for location in ShiftedMetric::to/fromFieldAligned.

[ZedThree]

This is now fixed