SurfaceRZFourier conversion to Desc format - #628
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds bidirectional conversion between SurfaceRZFourier and DESC’s FourierRZToroidalSurface, with a roundtrip unit test to validate geometry preservation.
Changes:
- Implement
SurfaceRZFourier.from_desc(...)to build a Simsopt surface from a DESC surface. - Implement
SurfaceRZFourier.to_desc()to export a Simsopt surface to DESC. - Add a DESC-enabled roundtrip test using VMEC input files.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/simsopt/geo/surfacerzfourier.py |
Adds to_desc / from_desc conversion logic and DESC imports/guards. |
tests/geo/test_surface_rzfourier.py |
Adds a DESC roundtrip test to validate conversion preserves geometry and metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @SimsoptRequires(DescFourierRZToroidalSurface is not None, "to_desc method requires Desc module") | ||
| def to_desc(self) -> DescFourierRZToroidalSurface: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #628 +/- ##
==========================================
+ Coverage 90.42% 90.46% +0.03%
==========================================
Files 84 84
Lines 17899 17953 +54
==========================================
+ Hits 16186 16242 +56
+ Misses 1713 1711 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| @unittest.skipIf(DescFourierRZToroidalSurface is None, "desc python extension is not installed") | ||
| def test_to_from_desc_roundtrip(self): | ||
| """Test that to_desc and from_desc correctly converts DESC surface back to simsopt.""" | ||
| import os |
There was a problem hiding this comment.
Move import os to top of file
| return simsopt_surface | ||
|
|
||
| @SimsoptRequires(DescFourierRZToroidalSurface is not None, "to_desc method requires Desc module") | ||
| def to_desc(self) -> DescFourierRZToroidalSurface: |
There was a problem hiding this comment.
How about add check_orientation=True as a kwarg to this method here that gets passed to line 638, so users have more control over whether the orientation is flipped or not.
There was a problem hiding this comment.
done. We default to check_orientation=False, so that from_desc(to_desc(...))) is the identity.
Summary
This PR introduces new methods to convert
SurfaceRZFourierobjects to and from the equivalent Desc format,FourierRZToroidalSurface. The methods are covered by unit tests which convert stellarator-symmetric and non-stellarator symmetric surfaces.New Methods
Two new methods have been added to the
SurfaceRZFourierclass:to_desc: converts an instance of a surface to the Desc format,from_desc: a class method that builds aSurfaceRZFourierobject from an existing DescFourierRZToroidalSurface.