Skip to content
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

Coordinates: Replacing sqrt(g_22) with JB #3027

Draft
wants to merge 2 commits into
base: refactor-coordinates
Choose a base branch
from

Conversation

bendudson
Copy link
Contributor

Aiming to consistently handle left-handed coordinate systems, where J is negative. These happen in the standard BOUT++ field-aligned coordinates when the poloidal field is negative.

Implemented on top of refactor-coordinates #2873

Aiming to consistently handle left-handed coordinate systems,
where J is negative. These happen in the standard BOUT++
field-aligned coordinates when the poloidal field is negative.
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

src/mesh/difops.cxx Show resolved Hide resolved
src/mesh/difops.cxx Show resolved Hide resolved
src/mesh/difops.cxx Show resolved Hide resolved
src/mesh/difops.cxx Show resolved Hide resolved
src/mesh/difops.cxx Show resolved Hide resolved
src/mesh/difops.cxx Show resolved Hide resolved
src/mesh/difops.cxx Show resolved Hide resolved
src/mesh/difops.cxx Show resolved Hide resolved
src/mesh/difops.cxx Show resolved Hide resolved
@ZedThree
Copy link
Member

Ah, this currently doesn't work because we've not added the overload for Bxy(x, y, z)

@dschwoerer
Copy link
Contributor

dschwoerer commented Nov 18, 2024 via email

@bendudson
Copy link
Contributor Author

Does this fix things for FCI where JB is not equal to sqrt(g22)? I wanted to discuss this with Ben on Wednesday.

Hey @dschwoerer I hope this will help with FCI too: I'm trying to remove all use of sqrt(g_22) from the code, and assumptions that J is positive. B is always positive (as it's the magnitude of B), but J can be positive or negative. In the tokamak case J < 0 when Bp < 0.

I haven't yet looked at the boundary conditions, or how to handle parallel components of vector quantities: When J < 0 it means that the y coordinate is in the opposite direction to B. Hence the 'y' component of a vector v is in the opposite direction to b dot v. It'll take some time to work out the implications for the operators e.g. Grad_par() takes a scalar argument, whereas Div_par() takes a vector component (b dot v).

@bendudson bendudson marked this pull request as draft November 18, 2024 21:38
@ZedThree
Copy link
Member

We also have some checks that J is positive that I guess need to be loosened?

@bendudson
Copy link
Contributor Author

We also have some checks that J is positive that I guess need to be loosened?

Ah yes. I think we should check that all elements of J have the same sign: All positive or all negative.

Copy link
Contributor

@dschwoerer dschwoerer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems some of the cases are (still) only valid for Clebsch.
I have not looked in too much detail, so if you disagree I can look in more detail ...

@@ -285,7 +286,7 @@ Field3D Div_par_flux(const Field3D& v, const Field3D& f, CELL_LOC outloc,
auto Bxy_floc = f.getCoordinates()->Bxy();

if (!f.hasParallelSlices()) {
return metric->Bxy() * FDDY(v, f / Bxy_floc, outloc, method) / sqrt(metric->g_22());
return FDDY(v, f / Bxy_floc, outloc, method) / metric->J();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it really be Bxy_floc? I thought this term comes from the curvi-linear geometry, and thus should be independent of B (for non clebsch)

Comment on lines 296 to +298
f_B.yup() = f.yup() / Bxy_floc;
f_B.ydown() = f.ydown() / Bxy_floc;
return metric->Bxy() * FDDY(v, f_B, outloc, method) / sqrt(metric->g_22());
return FDDY(v, f_B, outloc, method) / metric->J();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Comment on lines +482 to +483
result /= SQ(metric->J())
* metric->Bxy(); // J^2 B same sign for left & right handed coordinates
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move comment above to avoid strange formatting?

@@ -595,7 +597,7 @@ Field3D b0xGrad_dot_Grad(const Field3D& phi, const Field3D& A, CELL_LOC outloc)

Field3D result = VDDX(vx, A, outloc) + VDDY(vy, A, outloc) + VDDZ(vz, A, outloc);

result /= (metric->J() * sqrt(metric->g_22()));
result /= SQ(metric->J()) * metric->Bxy(); // J^2 B
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that not only true for Clebsch?

Copy link
Contributor

@dschwoerer dschwoerer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are some fixes for the compilation errors.

Alternative is to add the function to metric, like J and g_22 ...

BoutReal by = 1. / sqrt(metric->g_22(x, y, z));

// Note: by is negative in a left-handed coordinate system
BoutReal by = 1. / (metric->J(x, y, z) * metric->Bxy(x, y, z));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
BoutReal by = 1. / (metric->J(x, y, z) * metric->Bxy(x, y, z));
BoutReal by = 1. / (metric->J(x, y, z) * metric->Bxy()(x, y, z));

BoutReal const fluxRight =
fR * vR * (coord->J(i, j, k) + coord->J(i, j + 1, k))
/ (sqrt(coord->g_22(i, j, k)) + sqrt(coord->g_22(i, j + 1, k)));
fR * vR * 2 / (coord->Bxy(i, j, k) + coord->Bxy(i, j + 1, k));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fR * vR * 2 / (coord->Bxy(i, j, k) + coord->Bxy(i, j + 1, k));
fR * vR * 2 / (coord->Bxy()(i, j, k) + coord->Bxy()(i, j + 1, k));


// Calculate at left boundary (y-1/2)
BoutReal const fluxLeft =
fL * vL * (coord->J(i, j, k) + coord->J(i, j - 1, k))
/ (sqrt(coord->g_22(i, j, k)) + sqrt(coord->g_22(i, j - 1, k)));
fL * vL * 2 / (coord->Bxy(i, j, k) + coord->Bxy(i, j - 1, k));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fL * vL * 2 / (coord->Bxy(i, j, k) + coord->Bxy(i, j - 1, k));
fL * vL * 2 / (coord->Bxy()(i, j, k) + coord->Bxy()(i, j - 1, k));

@bendudson
Copy link
Contributor Author

It seems some of the cases are (still) only valid for Clebsch. I have not looked in too much detail, so if you disagree I can look in more detail ...

@dschwoerer I agree: The identity sqrt(g_22) = JB is only for Clebsch coordinates. In general how should the 'parallel' direction be defined? Perhaps we use sign(J) * sqrt(g_22) as the parallel metric, which would be consistent with the Clebsch case but I think more general. That would mean that if J > 0 then b is in the same direction as e_y, but if J < 0 then they are in opposite directions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants