Overhaul derivatives for vectorisation and user registration - #1384
Merged
Merged
Conversation
These are templated on the size of the offset to take and the direction in which to take the offset.
usable to populate any stencil
Can now use type deduction to avoid need to specify what FieldType is being used. Provides defaults to enable less typing in common uses
These just return a pointer to the parent field.
This is currently a bit ugly and enforces `nGuard == 1` even for Field2D which could in principle use more than one guard cell.
in index_derivs. Makes it clearer that the region is associated with iterating over the field. Removes one point of difference between 2D and 3D versions of index_deriv routines.
…templating_difops_trackingNext
…n' into more_templating_difops_trackingNext
Note appropriate type of field for template can be inferred from function signature and hence we don't need to explicitly specify this at the call sites
This partially reverts commit 591fe11. due to unintended code removal.
ZedThree
reviewed
Nov 30, 2018
ZedThree
reviewed
Nov 30, 2018
ZedThree
left a comment
Member
There was a problem hiding this comment.
Looks good, a few minor comments!
Member
|
Should we namespace some of the new things? The template combo stuff might go in one, though I've not got a name to mind. Possibly |
ZedThree
previously approved these changes
Nov 30, 2018
ZedThree
previously approved these changes
Nov 30, 2018
Removes them from being mesh members Rewrite approach for Flux:Split method to be implemented as all other derivatives.
ZedThree
reviewed
Nov 30, 2018
ZedThree
previously approved these changes
Nov 30, 2018
bendudson
approved these changes
Dec 4, 2018
Closed
This file contains hidden or 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
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.
Overhaul of index derivatives. Designed to provide the following main advantages:
Also provides the following minor changes:
uppercaseroutine.DDX,DDYandDDZwith various supported methods.std::stringinstead of aDIFF_METHODenum. This is required to allow for user registered routines. Thestd::stringmethods should be preferred and theDIFF_METHODoverloads may become deprecated.minusandplusoffsetting routine forSpecificIndwhich provides a simple way to offset in a requested direction with compile time information that works nicely with routines templated on theDIRECTIONenum class.fourthoption tomesh:dd?, currentlyC2is the only valid predefined option.Still to do:
Main implementation details:
meaning we have to effectively create an instance of the loop for every possible method and with all possible combinations of staggering, direction etc. This is achieving using some templating routines (in
include/bout/template_combinations.hxx) which can do "something" for a cartesian product of options.indexDD?routines now just call a single kernel (indexStandardDerivative) that gets the requested method and applies it in the appropriate direction. All theindexVDD?andindexFDD?routines now just call a single kernel (indexFlowDerivative) that gets the requested method and applies it.The performance examples suggest that most derivative methods now vectorise, giving performance comparable to direct hard-coded nested loop. Whilst this is around a factor 3-4 improvement over the current implementation in next this is not reflected in physics models due to the other costs involved in a full model.
Compilation of the library is likely to be slower due to the template combination expansion that must be done at compile time.