Fix some plot issues when NFP differs from one for objects, or when passed-in phi exceeds 2pi/nfp

[dpanici]

• in plot_surfaces (and plot_section and plot_boundary) when phi is supplied which exceeds NFP, the grid used for the map coordinates (a lineargrid with NFP=eq.NFP would truncate the phi. This is fine unless the new truncated phi has some duplicates. The grid is still a meshgrid of the supplied rho, theta, zeta, however, then the grid.num_zeta value, which gives the unique zeta values, might not correspond to the original length of phi. This causes reshaping errors like those seen in TypeError in 'plot_comparison' #1202. These grids though dont need to have the NFP arg at all as they always supply phi as an array (instead of supplying N), so we can set them to 1 to avoid this issie
• in plot_comparison and plot_boundaries, there is some ambiguity on what should happen by default when things of differing NFP are passed in. This changes to by default, throw an error if multiple nonaxisymmetric objects with difering NFP are passed in (as it is not clear what phis to do, and even then the plot title "phi*NFP/2pi" is ambiguous (which NFP does it refer to?)). If there are differing NFPs but the only ones with nonzero NFPs have the same NFP, and the remaining are axisymmetric objects, it will change the axisymmetric object NFP to match the nonaxisymetric ones (we can do that and it does not affect the plot since they are the same at every zeta).

The change in plot_boundaries behavior is something I am not 100% sure on, what do people think it should be? we could leave as is and just plot a bunch of XS, maybe with the phi being determined inside of the for loop instead of outside, so that if the default is used you get (0,2np.pi,4,endpoint=False). The only weird thing is now you have a bunch of XS where they are at differing angles and it is not clear how they should be compared to eachother.

Resolves #1202

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.33%. Comparing base (9f5cb55) to head (3223d50).
Report is 16 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1204   +/-   ##
=======================================
  Coverage   95.33%   95.33%           
=======================================
  Files          90       90           
  Lines       22699    22700    +1     
=======================================
+ Hits        21640    21642    +2     
+ Misses       1059     1058    -1     

Files	Coverage Δ
desc/plotting.py	95.73% <100.00%> (-0.08%)	⬇️

... and 2 files with indirect coverage changes

[github-actions]

|             benchmark_name             |         dt(%)          |         dt(s)          |        t_new(s)        |        t_old(s)        | 
| -------------------------------------- | ---------------------- | ---------------------- | ---------------------- | ---------------------- |
 test_build_transform_fft_lowres         |     +2.14 +/- 6.84     | +1.10e-02 +/- 3.51e-02 |  5.24e-01 +/- 3.4e-02  |  5.13e-01 +/- 8.8e-03  |
 test_build_transform_fft_midres         |     +0.55 +/- 3.63     | +3.27e-03 +/- 2.17e-02 |  6.01e-01 +/- 1.0e-02  |  5.98e-01 +/- 1.9e-02  |
 test_build_transform_fft_highres        |     -0.55 +/- 3.81     | -5.48e-03 +/- 3.81e-02 |  9.95e-01 +/- 2.4e-02  |  1.00e+00 +/- 2.9e-02  |
 test_equilibrium_init_lowres            |     +5.83 +/- 4.14     | +2.18e-01 +/- 1.54e-01 |  3.95e+00 +/- 1.5e-01  |  3.73e+00 +/- 3.5e-02  |
 test_equilibrium_init_medres            |     +0.15 +/- 3.59     | +6.12e-03 +/- 1.47e-01 |  4.11e+00 +/- 8.7e-02  |  4.10e+00 +/- 1.2e-01  |
 test_equilibrium_init_highres           |     +0.27 +/- 2.43     | +1.47e-02 +/- 1.32e-01 |  5.44e+00 +/- 9.6e-02  |  5.43e+00 +/- 9.1e-02  |
 test_objective_compile_dshape_current   |     +0.97 +/- 2.84     | +3.69e-02 +/- 1.08e-01 |  3.85e+00 +/- 1.0e-01  |  3.81e+00 +/- 2.8e-02  |
 test_objective_compile_atf              |     +0.21 +/- 1.88     | +1.66e-02 +/- 1.46e-01 |  7.81e+00 +/- 7.9e-02  |  7.79e+00 +/- 1.2e-01  |
 test_objective_compute_dshape_current   |     -1.36 +/- 1.71     | -4.72e-05 +/- 5.94e-05 |  3.42e-03 +/- 3.1e-05  |  3.47e-03 +/- 5.1e-05  |
 test_objective_compute_atf              |     -0.16 +/- 2.41     | -1.61e-05 +/- 2.47e-04 |  1.02e-02 +/- 2.1e-04  |  1.02e-02 +/- 1.3e-04  |
 test_objective_jac_dshape_current       |     +3.88 +/- 9.15     | +1.50e-03 +/- 3.53e-03 |  4.01e-02 +/- 1.7e-03  |  3.86e-02 +/- 3.1e-03  |
 test_objective_jac_atf                  |     -0.83 +/- 3.58     | -1.59e-02 +/- 6.82e-02 |  1.89e+00 +/- 3.4e-02  |  1.91e+00 +/- 5.9e-02  |
 test_perturb_1                          |     +0.94 +/- 1.05     | +1.14e-01 +/- 1.27e-01 |  1.22e+01 +/- 8.0e-02  |  1.21e+01 +/- 9.9e-02  |
 test_perturb_2                          |     +0.80 +/- 1.45     | +1.37e-01 +/- 2.48e-01 |  1.72e+01 +/- 2.3e-01  |  1.71e+01 +/- 8.2e-02  |
 test_proximal_jac_atf                   |     +0.25 +/- 1.15     | +2.00e-02 +/- 9.29e-02 |  8.07e+00 +/- 6.6e-02  |  8.05e+00 +/- 6.5e-02  |
 test_proximal_freeb_compute             |     -0.68 +/- 0.87     | -1.25e-03 +/- 1.61e-03 |  1.83e-01 +/- 8.1e-04  |  1.84e-01 +/- 1.4e-03  |
 test_proximal_freeb_jac                 |     -0.73 +/- 2.73     | -5.38e-02 +/- 2.02e-01 |  7.35e+00 +/- 6.9e-02  |  7.41e+00 +/- 1.9e-01  |
 test_solve_fixed_iter                   |     +0.82 +/- 61.16    | +3.98e-02 +/- 2.97e+00 |  4.89e+00 +/- 2.1e+00  |  4.85e+00 +/- 2.1e+00  |

[unalmis]

For your first point, isn't the root issue #1180?

Coordinates are not periodic functions, so we shouldn't treat them as such in the compute functions or constructing grids.

Here is my proposed solution:

1. Change LinearGrid to not mod $\zeta$ by $2 \pi / \text{NFP}$ and also not mod $\theta$ by $2 \pi$.
2. In LinearGrid, if the coordinates span multiple periods, that is if np.ptp($\zeta$) $> 2 \pi / \text{NFP}$ or np.ptp($\theta$) $> 2 \pi$, then set the attributes _spacing and _weights to None.

Reasoning:

1. Periodic functions should not be affected by this logic. This would be a bug fix for non-periodic functions.
2. Those attributes are quadrature weights for an integration over a geometry of NFP periods. When coordinates span a different length than that, one is effectively defining multiple copies of a volume or surface. We always want to compute an integral over NFP periods, so we modify the weights accordingly.
   In theory one could always modify the quadrature weights so that the integral is over the domain we want regardless of the coordinate span, but the API of LinearGrid is too broad to make that simple.

[dpanici]

Throw an error when NFP differs and say phi must be given explicitly

[YigitElma]

@dpanici Maybe we should change this to

not np.allclose([thing.NFP for thing in eqs], eqs[0].NFP) and (phi is None or isinstance(phi, numbers.Integral)),

Because integer phi is still ambigious.

[dpanici]

ah, what does it do rn if phi integral is passed in? btw 0->2pi or 0->NFP of the first thing?

[YigitElma]

It uses 0 to 2pi/nfp of the last thing.

[YigitElma]

Also, doesn't throw any error or warning.