ShiftedMetric with 2 yup/ydown points #1178
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Advective and flux derivatives, which take v and f as inputs, had cases using yup/ydown fields for one of v and f, but not the other. These do not make sense as multiplication of a field in field-aligned coordinates with another in non-field-aligned coordinates is incorrect. Delete these cases and fall back to converting both v and f to field-aligned if either does not have yup/ydown fields.
Remove the intermediate variable result_fa in branch of interp_to that transforms to field-aligned variables. Just store the intermediate result in 'result' instead, to save memory.
In several y-derivatives where we have to convert the input to field-aligned coordinates, the result was not transformed back to the original coordinates.
In the unstaggered case, only the value of v is used, so yup/ydown fields for v are not needed.
Was previously a typo that resulted in using non-field-aligned f in VDDY, which is incorrect.
Previously these were incorrectly setting the location of their result to the location of the input variable. This commit changes this to use the specified output location. This probably did not cause bugs as the location was overridden in DDX/DDY/DDZ, etc.
The location of the result needs to be set before shifting it from field-aligned coordinates, if this is necessary. So it is safer to set the location to diffloc (or outloc where diffloc is not yet set) as soon as 'Field3D result' is declared. There are a couple of exceptions where the location is set after calling apply*diff (which should not be necessary, but does not hurt, except that it may hide errors setting location in apply*diff) where the location of 'result' would not be used anyway earlier in the method.
Add region arguments (where it is valid to give RGN_NOX or RGN_NOBNDRY) so functions like toFieldAligned/fromFieldAligned can be used both when y-guard cells have been set and when they have not. e.g. when using fromFieldAligned on the results of interpolation or derivatives, y-guard cells cannot be included, but when using toFieldAligned on the inputs to interpolation or derivatives, y-guard cells must be included.
Replace yup_field and ydown_field with yup1_field, ydown1_field, yup2_field and ydown2_field. If MYG>1 (tested using fieldmesh->ystart>1) then calculate yup2_field and ydown2_field as well. This allows derivatives which require 5-point stencils when using shifted metric.
FCITransform does not support more than one yup/ydown field, so require it to use only one y-guard cell. Check for this in the constructor. Fix test-fci-slab to use myg=1.
Due to using static bool to cache phases, can only have one ShiftedMetric object. Therefore need to update test to use the ShiftedMetric in mesh instead of creating a new one.
Also add checks for locations of Field2D variables.
In ShiftedMetric, replace std::vector<dcomplex> with Array<dcomplex> and nested std::vector with Matrix< Array<dcomplex> >.
In ParallelTransform classes and in interp_to. This is necessary to make sure Flexible<> fields know the right location to give when they are given as arguments to arithmetic operators.
To ensure corner guard cells are not read, restrict all loops in ShiftedMetric to not include x-guard cells. Also add some extra calls to invalidateGuards to help catch future errors.
Using static bool to check for cached phases means the check is global so if two ShiftedMetric objects are created the one which calls 'get...' second will fail. This is unlikely in a normal simulation but might happen if there are multiple meshes, for example in tests or post-processing using boutcore.
|
Replaced with #1345 Leaving the branch for future reference -- @johnomotani feel free to delete if you don't think it's needed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add support for 2 yup/ydown points to ShiftedMetric, enabled when MYG>1. This allows higher-order derivative methods (4th order centred differences or 2nd/3rd order upwind schemes) to be used.
Also has some support for staggered grids. The original intention was to enable 4th order interpolation to be compatible with staggered grids, but this is not enough. To support staggered grids, ShiftedMetric would have to calculate interpolated values half a grid cell along the field from all the centred and staggered points. This does not seem practical, which motivated writing the ShiftToFieldAligned class in #1177.
I don't think this is actually useful to merge, since it adds quite a lot of complexity to ShiftedMetric but still does not support staggered grids. Uploading in case anyone might find it useful in future.