-
Notifications
You must be signed in to change notification settings - Fork 21
3D FCI support #308
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
3D FCI support #308
Conversation
* full_velocity does not work, so is skipped for now.
Co-authored-by: David Bold <[email protected]>
Co-authored-by: David Bold <[email protected]>
Co-authored-by: David Bold <[email protected]>
Co-authored-by: David Bold <[email protected]>
Co-authored-by: David Bold <[email protected]>
The extern YBoundary yboundary; has been removed from BOUT++, as it does not provide this global instance.
Minimum required to get it compiling with 3D metrics
Whatever used to be decleared in there is also in e.g. cassert defined, so we can just skip that.
returning const can break e.g. move operators
This is not implemented, so skip if for diagonistic only reasons
This reverts commit 4c285d3.
|
|
||
| bool legacy_match{true}; | ||
|
|
||
| Field3D fromFieldAligned(const Field3D& f) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these to/fromFieldAligned functions should be renamed: The caller may expect that the result is transformed, but if using FCI then it is not. That affects whether the caller can treat f(x, y+1, z) as being along a field line from f(x, y, z).
I'm coming to the opinion that trying to handle both yup/down and field-aligned cases with the same code is going to be fragile and easy to introduce errors. I think I would prefer a Mesh- or ParallelTransform-level boolean: Are we using yup/down fields for differencing or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these to/fromFieldAligned functions should be renamed: The caller may expect that the result is transformed, but if using FCI then it is not. That affects whether the caller can treat
f(x, y+1, z)as being along a field line fromf(x, y, z).
I do not think that should be a major issue.
If f(x, y+1, z) is not field aligned, then using f(x, y+1, z) with CHECK=4 will crash, as the boundary is NaN.
I'm coming to the opinion that trying to handle both yup/down and field-aligned cases with the same code is going to be fragile and easy to introduce errors.
If that is the route we want to go, then I think we need a hard fork of hermes-3.
I tried that with hermes-2, and the code is completely diverged.
I think I would prefer a Mesh- or ParallelTransform-level boolean: Are we using yup/down fields for differencing or not?
Maybe we just need more synthatitc sugar :-)
if f[i.yp()] would result into what is right now f.yup()[i.yp()] then I think more code would "just work".
If you think that is needed, I can try to implement that, it should not be that hard.
The
sheath_boundary_simple.cxxhas not been updated for a bit, but this shows how the boundary code can be simplified.