Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove basis kwarg from _compute function calls in magnetic field classes #1137

Merged
merged 3 commits into from
Jul 24, 2024

Conversation

dpanici
Copy link
Collaborator

@dpanici dpanici commented Jul 20, 2024

  • Fixes some leftover basis args inside of _compute calls, which now throw an error since that function only returns rpz quantities

Copy link

codecov bot commented Jul 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.23%. Comparing base (4b21abc) to head (6a2f604).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1137   +/-   ##
=======================================
  Coverage   95.23%   95.23%           
=======================================
  Files          87       87           
  Lines       21918    21920    +2     
=======================================
+ Hits        20873    20876    +3     
+ Misses       1045     1044    -1     
Files Coverage Δ
desc/coils.py 97.10% <100.00%> (+0.19%) ⬆️
desc/magnetic_fields/_current_potential.py 99.41% <100.00%> (+0.58%) ⬆️

... and 1 file with indirect coverage changes

Copy link
Contributor

github-actions bot commented Jul 20, 2024

|             benchmark_name             |         dt(%)          |         dt(s)          |        t_new(s)        |        t_old(s)        | 
| -------------------------------------- | ---------------------- | ---------------------- | ---------------------- | ---------------------- |
 test_build_transform_fft_lowres         |     +0.84 +/- 8.26     | +4.30e-03 +/- 4.24e-02 |  5.18e-01 +/- 3.9e-02  |  5.13e-01 +/- 1.6e-02  |
 test_build_transform_fft_midres         |     +0.12 +/- 6.32     | +7.20e-04 +/- 3.79e-02 |  6.01e-01 +/- 3.7e-02  |  6.00e-01 +/- 9.6e-03  |
 test_build_transform_fft_highres        |     +0.69 +/- 1.61     | +6.86e-03 +/- 1.59e-02 |  9.96e-01 +/- 1.1e-02  |  9.89e-01 +/- 1.2e-02  |
 test_equilibrium_init_lowres            |     -0.51 +/- 1.03     | -1.89e-02 +/- 3.80e-02 |  3.68e+00 +/- 3.3e-02  |  3.70e+00 +/- 1.9e-02  |
 test_equilibrium_init_medres            |     +0.35 +/- 0.82     | +1.46e-02 +/- 3.41e-02 |  4.16e+00 +/- 2.8e-02  |  4.14e+00 +/- 1.9e-02  |
 test_equilibrium_init_highres           |     -0.07 +/- 0.67     | -3.79e-03 +/- 3.73e-02 |  5.57e+00 +/- 2.4e-02  |  5.58e+00 +/- 2.8e-02  |
 test_objective_compile_dshape_current   |     -0.40 +/- 1.19     | -1.54e-02 +/- 4.56e-02 |  3.81e+00 +/- 3.0e-02  |  3.82e+00 +/- 3.4e-02  |
 test_objective_compile_atf              |     -0.76 +/- 2.93     | -6.20e-02 +/- 2.40e-01 |  8.10e+00 +/- 1.8e-01  |  8.16e+00 +/- 1.6e-01  |
 test_objective_compute_dshape_current   |     -0.17 +/- 3.83     | -2.17e-06 +/- 4.87e-05 |  1.27e-03 +/- 2.8e-05  |  1.27e-03 +/- 4.0e-05  |
 test_objective_compute_atf              |     +0.64 +/- 5.14     | +2.71e-05 +/- 2.18e-04 |  4.26e-03 +/- 1.4e-04  |  4.23e-03 +/- 1.7e-04  |
 test_objective_jac_dshape_current       |     -6.50 +/- 10.33    | -2.49e-03 +/- 3.96e-03 |  3.58e-02 +/- 2.7e-03  |  3.83e-02 +/- 2.9e-03  |
 test_objective_jac_atf                  |     +0.62 +/- 2.90     | +1.16e-02 +/- 5.42e-02 |  1.88e+00 +/- 3.2e-02  |  1.87e+00 +/- 4.4e-02  |
 test_perturb_1                          |     -0.05 +/- 2.22     | -7.12e-03 +/- 2.93e-01 |  1.32e+01 +/- 1.8e-01  |  1.32e+01 +/- 2.3e-01  |
 test_perturb_2                          |     +0.30 +/- 0.66     | +5.38e-02 +/- 1.19e-01 |  1.81e+01 +/- 5.8e-02  |  1.80e+01 +/- 1.0e-01  |
 test_proximal_jac_atf                   |     +0.01 +/- 2.79     | +8.56e-04 +/- 2.06e-01 |  7.38e+00 +/- 4.7e-02  |  7.38e+00 +/- 2.0e-01  |
-test_proximal_freeb_compute             |     +2.35 +/- 0.74     | +4.14e-03 +/- 1.30e-03 |  1.80e-01 +/- 8.8e-04  |  1.76e-01 +/- 9.6e-04  |
 test_proximal_freeb_jac                 |     -0.33 +/- 0.96     | -2.42e-02 +/- 7.10e-02 |  7.37e+00 +/- 3.3e-02  |  7.39e+00 +/- 6.3e-02  |
 test_solve_fixed_iter                   |     +0.30 +/- 3.38     | +5.51e-02 +/- 6.16e-01 |  1.83e+01 +/- 3.4e-01  |  1.82e+01 +/- 5.1e-01  |

f0uriest
f0uriest previously approved these changes Jul 20, 2024
Copy link
Collaborator

@ddudt ddudt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is wrong on master but wasn't being covered, we should add tests.

@dpanici
Copy link
Collaborator Author

dpanici commented Jul 23, 2024

If this is wrong on master but wasn't being covered, we should add tests.

Tests caught a bug, thanks for making me write them

@dpanici dpanici requested review from ddudt and f0uriest July 23, 2024 13:41
@dpanici dpanici merged commit e76f80e into master Jul 24, 2024
18 checks passed
@dpanici dpanici deleted the dp/hotfix-basis-compute-mag-field branch July 24, 2024 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants