Second order surfaces: order_r_option="r2" #4
Conversation
There was a problem hiding this comment.
For the new quasisymmetry_out files: It might be useful to have a different naming convention. It's unclear from the present names how these files differ from the older files in the repo like quasisymmetry_out.LandremanSengupta2019_section5.1.nc. Also "r2" in the name is ambiguous: it could refer either to which paper (Landreman & Sengupta 2019) or to the order by which the boundary was constructed. One option could be names like quasisymmetry_out.LandremanSengupta2019_section5.1_order_r2.nc or quasisymmetry_out.LandremanSengupta2019_section5.1_order_r2_r0.005.nc.
qsc/Frenet_to_cylindrical.py
Outdated
| @@ -7,9 +7,10 @@ | |||
| import numpy as np | |||
| from scipy.optimize import root_scalar | |||
|
|
|||
| def Frenet_to_cylindrical_residual_func(phi0, X_spline, Y_spline, phi_target,\ | |||
| def Frenet_to_cylindrical_residual_func(phi0, order, X_spline, Y_spline, Z_spline, phi_target,\ | |||
There was a problem hiding this comment.
When there are so many arguments to a function, it's easy to get errors from having arguments in the wrong order or having too many/few arguments. Instead of passing order and the last 7 splines as individual arguments, how about instead having an argument qsc which is specified as self when this function is called, so all the splines can be accessed via qsc.R0_func, qsc.tangent_phi_spline, etc?
qsc/Frenet_to_cylindrical.py
Outdated
| for j_phi in range(nphi_conversion): | ||
| # Solve for the phi0 such that r0 + X1 n + Y1 b has the desired phi | ||
| phi_target = phi_conversion[j_phi] | ||
| phi0_rootSolve_min = phi_target - 1.0 / self.nfp | ||
| phi0_rootSolve_max = phi_target + 1.0 / self.nfp | ||
| res = root_scalar(Frenet_to_cylindrical_residual_func, xtol=1e-15, rtol=1e-15, maxiter=1000,\ | ||
| args=(X_spline,Y_spline,phi_target, self.R0_func, self.normal_R_spline,\ | ||
| self.normal_phi_spline, self.binormal_R_spline, self.binormal_phi_spline),\ | ||
| args=(self.order, X_spline, Y_spline, Z_spline, phi_target, self.R0_func, self.normal_R_spline,\ |
There was a problem hiding this comment.
See earlier comment - we could pass self here instead of the last 7 arguments and self.order.
qsc/Frenet_to_cylindrical.py
Outdated
| final_R, final_z = Frenet_to_cylindrical_1_point(phi0_solution, arr) | ||
| self.binormal_R_spline(phi0_solution), self.binormal_phi_spline(phi0_solution), self.binormal_z_spline(phi0_solution),\ | ||
| self.tangent_R_spline(phi0_solution), self.tangent_phi_spline(phi0_solution), self.tangent_z_spline(phi0_solution)] | ||
| final_R, final_z = Frenet_to_cylindrical_1_point(phi0_solution, self.order, arr) |
There was a problem hiding this comment.
Instead of packing so many quantities into arr, where you have to be careful about the order, how about passing self to Frenet_to_cylindrical_1_point, e.g. final_R, final_z = Frenet_to_cylindrical_1_point(phi0_solution, self). Then in Frenet_to_cylindrical_1_point, all the splines can be evaluated at phi0_solution.
qsc/Frenet_to_cylindrical.py
Outdated
| """ | ||
| sinphi0 = np.sin(phi0) | ||
| cosphi0 = np.cos(phi0) | ||
| X_at_phi0, Y_at_phi0, R0_at_phi0, z0_at_phi0,\ | ||
| X_at_phi0, Y_at_phi0, Z_at_phi0, R0_at_phi0, z0_at_phi0,\ |
There was a problem hiding this comment.
It looks like the order in which quantities are unpacked from arr here does not match the docstring above. This may be a moot point if the function arguments are refactored as described in another comment.
This pull request extends the previous pull request "Output to VMEC" to second order surfaces.
As for the moment, the implementation simply follows the simple recipe for the near-axis expansion at second order without any third order calculations. However, the goal is to be able to use the equivalent equations implemented in the Fortran code quasisymmetry when the flag order_r_option="r3_flux_constraint" is given in the the input file.
The fortran files with order_r_option="r2" were added for the O(r^2) paper configurations and the tests pass.