-
Notifications
You must be signed in to change notification settings - Fork 27
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
flip_theta
compatibility function
#1248
Conversation
| benchmark_name | dt(%) | dt(s) | t_new(s) | t_old(s) |
| -------------------------------------- | ---------------------- | ---------------------- | ---------------------- | ---------------------- |
test_build_transform_fft_midres | -0.56 +/- 5.71 | -3.38e-03 +/- 3.45e-02 | 6.01e-01 +/- 2.8e-02 | 6.05e-01 +/- 2.0e-02 |
test_build_transform_fft_highres | -1.89 +/- 3.78 | -1.91e-02 +/- 3.82e-02 | 9.93e-01 +/- 2.9e-02 | 1.01e+00 +/- 2.5e-02 |
test_equilibrium_init_lowres | +1.06 +/- 4.85 | +4.08e-02 +/- 1.87e-01 | 3.90e+00 +/- 1.3e-01 | 3.86e+00 +/- 1.3e-01 |
test_objective_compile_atf | +0.59 +/- 2.92 | +4.60e-02 +/- 2.29e-01 | 7.89e+00 +/- 2.1e-01 | 7.84e+00 +/- 8.4e-02 |
test_objective_compute_atf | -3.24 +/- 2.44 | -3.38e-04 +/- 2.55e-04 | 1.01e-02 +/- 1.1e-04 | 1.04e-02 +/- 2.3e-04 |
test_objective_jac_atf | +1.84 +/- 3.68 | +3.45e-02 +/- 6.90e-02 | 1.91e+00 +/- 4.9e-02 | 1.87e+00 +/- 4.9e-02 |
test_perturb_1 | +1.61 +/- 3.80 | +1.98e-01 +/- 4.66e-01 | 1.25e+01 +/- 3.9e-01 | 1.23e+01 +/- 2.5e-01 |
test_proximal_jac_atf | -0.10 +/- 1.11 | -8.25e-03 +/- 8.98e-02 | 8.08e+00 +/- 6.2e-02 | 8.08e+00 +/- 6.5e-02 |
test_proximal_freeb_compute | -0.72 +/- 1.53 | -1.32e-03 +/- 2.78e-03 | 1.81e-01 +/- 2.3e-03 | 1.82e-01 +/- 1.6e-03 |
test_build_transform_fft_lowres | +0.26 +/- 5.79 | +1.42e-03 +/- 3.15e-02 | 5.45e-01 +/- 2.7e-02 | 5.44e-01 +/- 1.6e-02 |
test_equilibrium_init_medres | -1.61 +/- 4.27 | -6.98e-02 +/- 1.85e-01 | 4.27e+00 +/- 3.8e-02 | 4.34e+00 +/- 1.8e-01 |
test_equilibrium_init_highres | -1.14 +/- 2.13 | -6.50e-02 +/- 1.21e-01 | 5.64e+00 +/- 9.2e-02 | 5.70e+00 +/- 7.9e-02 |
test_objective_compile_dshape_current | -0.61 +/- 0.85 | -2.39e-02 +/- 3.35e-02 | 3.90e+00 +/- 1.8e-02 | 3.92e+00 +/- 2.8e-02 |
test_objective_compute_dshape_current | -1.22 +/- 3.19 | -4.29e-05 +/- 1.12e-04 | 3.47e-03 +/- 7.6e-05 | 3.51e-03 +/- 8.3e-05 |
test_objective_jac_dshape_current | +1.15 +/- 7.64 | +4.64e-04 +/- 3.08e-03 | 4.08e-02 +/- 2.1e-03 | 4.03e-02 +/- 2.2e-03 |
test_perturb_2 | +0.50 +/- 1.81 | +8.95e-02 +/- 3.24e-01 | 1.80e+01 +/- 2.5e-01 | 1.79e+01 +/- 2.1e-01 |
test_proximal_freeb_jac | -0.57 +/- 1.37 | -4.32e-02 +/- 1.03e-01 | 7.51e+00 +/- 6.5e-02 | 7.55e+00 +/- 8.0e-02 |
test_solve_fixed_iter | -1.01 +/- 61.32 | -5.06e-02 +/- 3.07e+00 | 4.95e+00 +/- 2.2e+00 | 5.00e+00 +/- 2.2e+00 | |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1248 +/- ##
==========================================
+ Coverage 95.43% 95.44% +0.01%
==========================================
Files 95 95
Lines 23419 23419
==========================================
+ Hits 22350 22353 +3
+ Misses 1069 1066 -3
|
The tests are insufficient for checking is theta flip is correct. We should at least test the field aligned quantities used for GX, ripple, Gamma_c, etc. |
I feel dumb to ask this, but when I plot the flipped eq and original eq, they have the same flux surfaces. Is this because of the from desc.examples import get
from desc.plotting import plot_comparison
from desc.compat import flip_theta
eq = get("ATF")
eq0 = eq.copy()
eq_flip = flip_theta(eq0)
plot_comparison(eqs=[eq, eq_flip]); The compute function works expectedly, grid = LinearGrid(theta=6)
R_eq = eq.compute("R",grid=grid)["R"]
R_eq_flip = eq_flip.compute("R",grid=grid)["R"]
print(R_eq)
print(R_eq_flip)
|
This is the expected behavior, the equilibrium is physically the same and we are only changing the definition of |
This has nothing to do with field-aligned quantities. For stellarator symmetry, there are 2 unique boundary inputs that give the same physical equilibrium: This new function is very similar to the existing |
return eq | ||
|
||
rone = np.ones_like(eq.R_lmn) | ||
rone[eq.R_basis.modes[:, 1] % 2 == 1] *= -1 |
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.
Does this not flip the theta sign too?
I am confused, do you want this to only shift theta, or shift and flip?
simple example to illustrate my confusion:
R = R0 + a cos t
Z = -a sin t
is a CW poloidal angle circular torus
after flip_theta(eq) we get
R = R0 - a cos(t_new)
Z = -a sin(t_new)
now this is a CCW poloidal angle.
So what you have rn is
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.
No it does not flip the sign of theta. This flips the sign of all odd m modes, including both sine and cosine modes. So in your example, the final Z boundary will also flip sign to:
Z = +a sin(t_new)
and then the poloidal angle still increases clockwise.
The tests check that the Jacobian
Redefines$\theta_{new} = \theta_{old} + \pi$ by flipping the sign of all $R$ , $Z$ , $\lambda$ coefficients with odd $m$ .
This is useful for stuff like omnigenity and TERPSICHORE that assume$\theta=0$ is on the inboard/outboard side.