Add 4th order derivative transforms

[unalmis]

• Adds Zernike radial polynomial fourth order derivative. This is needed for Magnetic axis limits and new variables #556 .
• Ignore division by zero in compute funs.
• Update compute/_core with dummy omega function

[f0uriest]

The method setter is recursive, not sure what's causing the problem

…

On Tue, Jul 18, 2023, 7:15 PM Kaya Unalmis ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In desc/transform.py <#586 (comment)>: > self._built = False self._built_pinv = False - self._set_up() + self._derivatives = self._get_derivatives(derivs) + self._sort_derivatives() + # assign according to logic in setter function + self.method = method + # assign according to logic in property function + self._matrices = self.matrices Is the def method setter supposed to be recursive? If you notice it calls itself because it assigns with self.method = rather than self._method. "Fixing" this to make it not recursive generates similar missing attribute errors. It's not clear to me why either of these issues occur from making these changes, but maybe they are related. I'm hesitant to make your change. Currently it seems the matrices attribute is getting deleted after *init*, and I think your suggestion will avoid that by constantly reconstructing it whenever its called. — Reply to this email directly, view it on GitHub <#586 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFMFUIRVBM45JTVZLLTZJY3XQ4KI7ANCNFSM6AAAAAA2NPWS74> . You are receiving this because you commented.Message ID: ***@***.***>

[github-actions]

|             benchmark_name             |        dt(%)        |        dt(s)        |      t_new(s)      |      t_old(s)      | 
| ------------------------------- | ------------------- | ------------------- | ------------------ | ------------------ |
+test_build_transform_fft_lowres        | -1.38+/-0.86 | -1.67e-04+/-1.0e-04 | 1.20e-02+/-7.4e-05 | 1.21e-02+/-7.3e-05 |
+test_build_transform_fft_midres        | -2.23+/-0.51 | -2.40e-03+/-5.5e-04 | 1.05e-01+/-4.1e-04 | 1.08e-01+/-3.6e-04 |
+test_build_transform_fft_highres       | -1.76+/-0.54 | -9.08e-03+/-2.8e-03 | 5.06e-01+/-2.6e-03 | 5.15e-01+/-1.0e-03 |
 test_equilibrium_init_lowres           | -1.17+/-1.39 | -5.73e-03+/-6.8e-03 | 4.86e-01+/-6.8e-03 | 4.92e-01+/-5.9e-04 |
+test_equilibrium_init_medres           | -1.40+/-0.40 | -3.24e-02+/-9.4e-03 | 2.28e+00+/-5.0e-03 | 2.32e+00+/-7.9e-03 |
+test_equilibrium_init_highres          | -0.70+/-0.32 | -5.38e-02+/-2.5e-02 | 7.68e+00+/-1.3e-02 | 7.73e+00+/-2.1e-02 |
+test_objective_compile_heliotron       | -1.67+/-1.11 | -1.96e-01+/-1.3e-01 | 1.16e+01+/-6.7e-02 | 1.17e+01+/-1.1e-01 |
 test_objective_compile_dshape_current  | -3.45+/-4.47 | -1.40e-01+/-1.8e-01 | 3.91e+00+/-1.2e-01 | 4.05e+00+/-1.3e-01 |
 test_objective_compile_atf             | -1.17+/-2.19 | -1.44e-01+/-2.7e-01 | 1.22e+01+/-1.7e-01 | 1.23e+01+/-2.1e-01 |
 test_objective_compute_heliotron       | -2.45+/-353.25 | -6.01e-04+/-8.7e-02 | 2.39e-02+/-6.0e-02 | 2.45e-02+/-6.2e-02 |
 test_objective_compute_dshape_current  | -1.45+/-416.78 | -1.59e-04+/-4.6e-02 | 1.08e-02+/-3.2e-02 | 1.10e-02+/-3.3e-02 |
 test_objective_compute_atf             | -1.31+/-368.22 | -3.07e-04+/-8.6e-02 | 2.31e-02+/-6.0e-02 | 2.34e-02+/-6.1e-02 |
-test_objective_jac_heliotron           | +15.97+/-0.41 | +7.83e-01+/-2.0e-02 | 5.68e+00+/-1.3e-02 | 4.90e+00+/-1.6e-02 |
 test_objective_jac_dshape_current      | -2.42+/-4.55 | -2.93e-03+/-5.5e-03 | 1.18e-01+/-2.0e-03 | 1.21e-01+/-5.1e-03 |
 test_objective_jac_atf                 | -0.44+/-0.90 | -2.33e-02+/-4.8e-02 | 5.31e+00+/-2.3e-02 | 5.33e+00+/-4.2e-02 |
 test_perturb_1                         | -0.56+/-45.17 | -4.79e-02+/-3.8e+00 | 8.47e+00+/-2.7e+00 | 8.52e+00+/-2.7e+00 |
 test_perturb_2                         | -0.59+/-3.01 | -8.70e-02+/-4.4e-01 | 1.46e+01+/-3.0e-01 | 1.47e+01+/-3.2e-01 |

[codecov]

Codecov Report

Merging #586 (d76d9dd) into master (d896b0a) will increase coverage by 0.07%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #586      +/-   ##
==========================================
+ Coverage   94.14%   94.21%   +0.07%     
==========================================
  Files          74       74              
  Lines       17426    17646     +220     
==========================================
+ Hits        16405    16626     +221     
+ Misses       1021     1020       -1     

Files Changed	Coverage Δ
desc/compute/_core.py	100.00% <ø> (ø)
desc/io/equilibrium_io.py	85.89% <ø> (ø)
desc/basis.py	97.82% <100.00%> (+0.03%)	⬆️
desc/transform.py	91.30% <100.00%> (+0.37%)	⬆️

[dpanici]

could you add a correctness test for the jacobi derivative 4th order? like some crude check against finite differences or something, just to make sure things are correct.

[unalmis]

Made requested change to merge some changes from #556

[f0uriest]

Is there any particular order to the entries in this file? I think at one point they were sorted alphabetically by derivative order and I'd like to keep that if possible to make code generation and maintenance easier.

[unalmis]

They have now been sorted using the script I made here: https://github.com/PlasmaControl/DESC/blob/basis_vectors/docs/sort_compute_funs.py

[unalmis]

It looks like sorting them alphabetically doubled the diff size. If you want a custom sort order with lower case first that can be done via the one-line modification to the script as discussed here https://stackoverflow.com/a/28136499.

[f0uriest]

Thanks! The actual order isn't so important as long as there is some logic to it so its reproducible and we can keep it in order.