Add API to normalise metrics - #3463
Conversation
…ormalise-metrics
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Is this different from Could this also be a free function, rather than a member of |
Yes, this is from the Hermes-3 branch that uses the metrics provided, and just normalises them.
No, because calling the setter on the metric tensor does recompute J and B. This is very wrong for FCI, and even for non-FCI not wanted. We want to just rescale those quantities. |
It has never been implemented
|
Ah, that's because we forgot to add an overload to |
| if (Bxy().isFci()) { | ||
| g_22_ylow(); | ||
| g_22_yhigh(); | ||
| ASSERT2(_g_22_ylow.has_value()); | ||
| (*_g_22_ylow) /= rhoSQ; | ||
| ASSERT2(_g_22_yhigh.has_value()); | ||
| (*_g_22_yhigh) /= rhoSQ; | ||
| } |
There was a problem hiding this comment.
@ZedThree But then we need to expose an API for this?
I really think we should not do that.
There was a problem hiding this comment.
What if we move them to MetricTensor, and then TokamakCoordinates can also take care of normalising them?
There was a problem hiding this comment.
We could, but that surely should be FCICoordinates?
Are we fine with MetricTensor having a pointer to the mesh, so that it can load from the grid?
There was a problem hiding this comment.
I don't follow, sorry. g_22_ylow/high are computed for both FCI and non-FCI, and this normalisation is a particular tokamak one rather than FCI specific.
We could do that, or we could just load it along with the rest of the metric if it exists?
There was a problem hiding this comment.
I don't follow, sorry.
g_22_ylow/highare computed for both FCI and non-FCI,
Somewhat. For FCI, zoidberg does the computation using the actually magnetic field line length. On the BOUT++ side, it just gets loaded. For non-FCI we just interpolate.
and this normalisation is a particular tokamak one rather than FCI specific.
It needs to be done for the FCI, as we load it, rather then interpolate. And I think I also put it correctly in the FCI branch.
We could do that, or we could just load it along with the rest of the metric if it exists?
So you want to not do the lazy loading of that anymore? Should we then always throw for FCI if it is missing from the grid, even if the g_22_ylow is not used?
There was a problem hiding this comment.
If we move this to the metric and keep it lazy, the metric now needs to now about the parallel transform.
I am wondering whether all the extra work is worth it? Should we maybe just accept that it is probably easier to have the normalisation done by the coordinates, and have an argument to select what kind of normalisation you want? Then it could not be done by a derived class without issues, but we could still offer the user a choice beyond the FCI and Tokamak geometries?
2 participants
With the encapsulation of the metrics, it got a bit more annoying to normalise the metrics.
Doing it in BOUT++ simplifies user code, but still allowing to do it in different ways, if you want something else.