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

CurveXYZFourierSymmetries #404

Merged
merged 22 commits into from
Apr 29, 2024
Merged

CurveXYZFourierSymmetries #404

merged 22 commits into from
Apr 29, 2024

Conversation

andrewgiuliani
Copy link
Contributor

@andrewgiuliani andrewgiuliani commented Apr 18, 2024

This pull request introduces a new helical curve representation called CurveXYZHelical. This curve is different from CurveXYZFourier because it can have discrete rotational symmetry. It can also be stellarator symmetric, or not.

We currently support a helical curve representation CurveHelical, but this curve is restricted to lie on a torus. The new CurveXYZHelical is not restricted to lie on a torus, and can represent curves like the one below. Viewing from above, the CurveXYZHelical appears to lie on a torus
helical_top

but profile view reveals that it clearly does not:
helical_side

When CurveXYZHelical is stellarator symmetric, then necessarily the curve must pass through the point (x0, 0, 0). If you do not want this, then the user can apply stellarator symmetry to a non-stellarator symmetric CurveXYZHelical curve and obtain a stellsym magnetic field from those two curves.

Copy link

codecov bot commented Apr 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.60%. Comparing base (d830719) to head (6841058).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #404      +/-   ##
==========================================
+ Coverage   91.55%   91.60%   +0.04%     
==========================================
  Files          74       75       +1     
  Lines       12911    12987      +76     
==========================================
+ Hits        11821    11897      +76     
  Misses       1090     1090              
Flag Coverage Δ
unittests 91.60% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@landreman landreman left a comment

Choose a reason for hiding this comment

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

Here is a small generalization you could consider: in the transformation from \hat{x},\hat{y} to x,y, the angle in the rotation matrix could be multiplied by some integer. This would allow for curves for which you need to go around multiple times toroidally before they close. Not critical, just an idea.

tests/geo/test_curve.py Outdated Show resolved Hide resolved
@andrewgiuliani
Copy link
Contributor Author

Here is a small generalization you could consider: in the transformation from \hat{x},\hat{y} to x,y, the angle in the rotation matrix could be multiplied by some integer. This would allow for curves for which you need to go around multiple times toroidally before they close. Not critical, just an idea.

sounds like a good idea, I'll add this to the PR in the coming days.

@andrewgiuliani andrewgiuliani changed the title CurveXYZHelical CurveXYZFourierSymmetries Apr 22, 2024
@andrewgiuliani
Copy link
Contributor Author

I've introduced support for curves to wrap toroidally ntor times before biting its tail. This allows us to represent some more interesting curves with nfp-fold discrete rotational symmetry that pass ntor times toroidally before biting their tail. In addition to helical coils, we can now represent knots such as a trefoil:

trefoil

This trefoil has nfp=3 and ntor=2. I've added a unit test to verify this in geo.test_curve.Testing.test_trefoil_stellsym and geo.test_curve.Testing.test_trefoil_nonstellsym.

A few things to note:

  1. Since this is a much larger class of curves, i.e., not just helical curves, I've renamed the class to be CurveXYZFourierSymmetries, reflecting that the class has symmetries baked in (rotational and stellarator symmetries), that CurveXYZFourier does not.

  2. Note that ntor and nfp must be coprime, or else the actual nfp of the curve is nfp//gcd(nfp,ntor) and the actual ntor of the curve is ntor//gcd(nfp, ntor). To avoid confusion, we assert that gcd(nfp, ntor)=1 at instantiation of the class.

  3. When ntor=1 the usual relation showing rotational symmetry holds, i.e. gamma(theta + 1/nfp) = R(1/nfp) * gamma(theta), where R(alpha) is a rotation matrix. When ntor>1, a more general relation holds gamma(theta + 1/nfp) = R(ntor/nfp) * gamma(theta). This still results in a curve with nfp-fold discrete rotational symmetry.

@landreman landreman self-requested a review April 24, 2024 17:58
Copy link
Contributor

@rogeriojorge rogeriojorge left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@andrewgiuliani andrewgiuliani merged commit 295ac65 into master Apr 29, 2024
46 of 47 checks passed
@andrewgiuliani andrewgiuliani deleted the ag/curvexyzhelical branch April 29, 2024 17:17
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.

None yet

4 participants