Cbs/fieldine integrator

[smiet]

Revamp of fieldline integration. Some tests still need to be written, but this should simplify the user experience in generating Poincare plots.

What’s new

Integrator base class
with subclasses:

• SimsoptFieldlineIntegrator (interface to the sopp integration routines
• ScipyFieldlineIntegrator (integration of the magnetic field using scipy.solve_ivp methods).

These subclasses provide methods to return Poincare sections, three dimensional field line trajectories, as well as the ability to integrate from any starting point over a given toroidal distance.

The ScipyFieldlineIntegrator provides methods to integrate $R$ and $Z$ using $\phi$ as the 'time' variable in the ODE, providing crisper Poincare sections and integrates about as fast as the sopp methods (which evolves the three coordinates independently).

The Integrator subclasses also provide an interface to the PoincarePlotter class, which provides an easy interface to create Poincare plots of the field.

The PoincarePlotter uses caching and the simsopt dependency graph (parent Integrator with parent MagneticField) to trigger recomputes when necessary.
Manipulation of the res_phi_hits and res_tys arrays is a thing of the past!

give it a spin!:

from simsopt.configs import get_data
from simsopt.field import SimsoptFieldlineIntegrator, PoincarePlotter
from simsopt.geo import plot
import numpy as np

base_coils, base_currents, ma, nfp, bs = get_data('ncsx')

axis_rz = ma.gamma()[0][::2]
# startpoints linspace from axis out: 
start_points_RZ = np.linspace(axis_rz, axis_rz+np.array([0.1, 0]), 10)


integrator = SimsoptFieldlineIntegrator(bs, stellsym=True, nfp=nfp, R0=axis_rz[0], tol=1e-9)
pplotter = PoincarePlotter(integrator, start_points_RZ, n_transits=200, phis=4)

fig, axs = pplotter.plot_poincare_all()


pplotter.plot_fieldline_trajectories_3d(engine='mayavi', show=False, opacity=0.2, tube_radius=0.001)
plot(bs.coils, engine='mayavi', show=False)
pplotter.plot_poincare_in_3d(engine='mayavi', show=True, scale_factor=0.01)

[codecov]

Codecov Report

❌ Patch coverage is 90.75044% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.17%. Comparing base (f8c9be3) to head (9ac531c).

Files with missing lines	Patch %	Lines
src/simsopt/field/tracing.py	90.75%	53 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #555      +/-   ##
==========================================
+ Coverage   90.14%   90.17%   +0.02%     
==========================================
  Files          84       84              
  Lines       17880    18451     +571     
==========================================
+ Hits        16118    16638     +520     
- Misses       1762     1813      +51     

Flag	Coverage Δ
unittests	90.17% <90.75%> (+0.02%)	⬆️

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.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[smiet]

@andrewgiuliani @mishapadidar @landreman Think we're good to go! lmk if more changes are needed.

[mishapadidar]

Hi @smiet, just checking in on this PR. Is this still in progress? It looks like there are still comments to be adressed in the code.

[smiet]

Hi @mishapadidar I left comments under your comments, but I did not 'resolve' them. I feel happy with the current status and want to move forward, so I now resolved all. In big lines, your asks that we need to discuss are:

naming, and 'duplicate' functions

I have functions like integrate_<what>_<inputconvention> (f.ex. integrate_fieldlinepoints_cart)
I think having all of these functions is extremely useful and outweighs the duplication because:

• The duplication is minor, the extra functions consist of a few lines to apply the right coordinate transform and are 80% comment
• For the user this is so much more convenient. Do you know the axis in cylindrical coordinates? just use the <>_cyl function, are you provided cartesian from calling gamma()? just as simple.
• I am planning expansions that underlie this choice: Want to generate a SurfaceRZFourier from your fieldlne? use the return_cartesian = True. RZFourier? ask the return in cylindrical. This prevents all this transformation code from being duplicated in the places it is used.
• This aligns with the simsoptpp convention where the _cyl suffix determines the return coordinates (e.g. B_cyl, A_cyl).

compute_poincare_trajectories return signature mismatch

ScipyFieldlineIntegrator and SimsoptFieldlineIntegrator handle computation of the Poincare hits differently, and that the ScipyFieldlineIntegrator has two calls, one for the trajectories, one for the plane hits.

This is because of how the ODE is solved, the simsoptpp call to boost automatically returns both, but you get better faster results with scipy (because you are using \phi as integration coordinate) in the way I programmed it. I want to keep this as efficient as possible, so I do not want to calculate full trajectories each call.

These functions are to only be called by PoincarePlotter, and there is specific logic to handle this there.

If I add a kwarg, I still want the SimsoptFieldlineIntegrator to return both values, because it is always computed and would be a waste not to cache it. this would add a useless kwarg to that function, but it would make the interface simpler. I opted to put the logic in PoincarePlotter, but if you want me to I can switch it to the integrator call, LMK.

other

• I did not address you suggestion to use xyz0 or RZ0 instead of start_xyz and start_RZ. If you want me to, I will do this re-factor, lmk.

[smiet]

@mishapadidar can we move forward on this PR?

[andrewgiuliani]

Hi @smiet, unit tests are failing here

[smiet]

should be better now, @mishapadidar

[smiet]

@mishapadidar @andrewgiuliani can we move on this? I have other developments building on this PR and I want to confirm that the call signature and name integrate_toroidally are an acceptable resolution to our discussion (former integrate_in_phi_cyl).

[smiet]

I am sorry to be such a nag, but can we please move on this? @mishapadidar @andrewgiuliani