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

ε and Γ_c with Fourier Bounce #1290

Merged
merged 119 commits into from
Dec 24, 2024
Merged

ε and Γ_c with Fourier Bounce #1290

merged 119 commits into from
Dec 24, 2024

Conversation

unalmis
Copy link
Collaborator

@unalmis unalmis commented Oct 3, 2024

This PR updates the

objectives to use the numerical methods from #1119

Also includes some incremental performance improvements

@unalmis unalmis added the easy Short and simple to code or review label Oct 3, 2024
@unalmis unalmis changed the base branch from master to ku/fourier_bounce October 3, 2024 05:56
@unalmis unalmis changed the title Writing neoclassical functions to use pseudospectral methods Writing ε¹ᐧ⁵ and Γ_c compute funs with new methods Oct 3, 2024
@unalmis unalmis mentioned this pull request Oct 3, 2024
@unalmis unalmis changed the title Writing ε¹ᐧ⁵ and Γ_c compute funs with new methods Writing ε and Γ_c compute funs with new methods Oct 3, 2024
Copy link
Contributor

github-actions bot commented Oct 3, 2024

|             benchmark_name             |         dt(%)          |         dt(s)          |        t_new(s)        |        t_old(s)        | 
| -------------------------------------- | ---------------------- | ---------------------- | ---------------------- | ---------------------- |
 test_build_transform_fft_lowres         |     +3.05 +/- 9.20     | +1.56e-02 +/- 4.73e-02 |  5.29e-01 +/- 4.7e-02  |  5.14e-01 +/- 6.8e-03  |
 test_equilibrium_init_medres            |     +2.02 +/- 2.52     | +8.18e-02 +/- 1.02e-01 |  4.13e+00 +/- 9.8e-02  |  4.04e+00 +/- 2.9e-02  |
 test_equilibrium_init_highres           |     +1.40 +/- 2.54     | +7.45e-02 +/- 1.35e-01 |  5.39e+00 +/- 1.2e-01  |  5.31e+00 +/- 5.6e-02  |
 test_objective_compile_dshape_current   |     -0.25 +/- 0.82     | -9.62e-03 +/- 3.21e-02 |  3.88e+00 +/- 2.1e-02  |  3.89e+00 +/- 2.5e-02  |
 test_objective_compute_dshape_current   |     +0.88 +/- 2.14     | +4.49e-05 +/- 1.10e-04 |  5.16e-03 +/- 1.0e-04  |  5.11e-03 +/- 3.5e-05  |
 test_objective_jac_dshape_current       |     -1.55 +/- 6.11     | -6.69e-04 +/- 2.63e-03 |  4.24e-02 +/- 1.7e-03  |  4.30e-02 +/- 2.0e-03  |
 test_perturb_2                          |     -0.01 +/- 1.01     | -2.69e-03 +/- 1.95e-01 |  1.94e+01 +/- 1.0e-01  |  1.94e+01 +/- 1.7e-01  |
 test_proximal_freeb_jac                 |     -0.41 +/- 1.13     | -3.02e-02 +/- 8.28e-02 |  7.32e+00 +/- 4.8e-02  |  7.35e+00 +/- 6.8e-02  |
 test_solve_fixed_iter                   |     +0.19 +/- 1.74     | +5.86e-02 +/- 5.45e-01 |  3.13e+01 +/- 4.3e-01  |  3.13e+01 +/- 3.3e-01  |
 test_LinearConstraintProjection_build   |     -0.79 +/- 2.92     | -7.98e-02 +/- 2.96e-01 |  1.00e+01 +/- 2.0e-01  |  1.01e+01 +/- 2.2e-01  |
 test_build_transform_fft_midres         |     +4.68 +/- 3.50     | +2.81e-02 +/- 2.11e-02 |  6.30e-01 +/- 2.0e-02  |  6.02e-01 +/- 7.4e-03  |
 test_build_transform_fft_highres        |     +1.65 +/- 2.35     | +1.60e-02 +/- 2.29e-02 |  9.88e-01 +/- 2.0e-02  |  9.72e-01 +/- 1.0e-02  |
 test_equilibrium_init_lowres            |     +5.18 +/- 3.07     | +1.97e-01 +/- 1.16e-01 |  3.99e+00 +/- 1.1e-01  |  3.79e+00 +/- 3.3e-02  |
 test_objective_compile_atf              |     -0.07 +/- 1.90     | -5.81e-03 +/- 1.55e-01 |  8.12e+00 +/- 1.2e-01  |  8.13e+00 +/- 9.9e-02  |
 test_objective_compute_atf              |     +0.07 +/- 2.36     | +1.17e-05 +/- 3.73e-04 |  1.58e-02 +/- 2.7e-04  |  1.58e-02 +/- 2.6e-04  |
 test_objective_jac_atf                  |     -0.66 +/- 2.43     | -1.33e-02 +/- 4.86e-02 |  1.99e+00 +/- 4.2e-02  |  2.00e+00 +/- 2.4e-02  |
 test_perturb_1                          |     +0.14 +/- 1.33     | +2.08e-02 +/- 1.94e-01 |  1.46e+01 +/- 1.6e-01  |  1.46e+01 +/- 1.1e-01  |
 test_proximal_jac_atf                   |     +0.46 +/- 0.82     | +3.76e-02 +/- 6.78e-02 |  8.27e+00 +/- 4.1e-02  |  8.23e+00 +/- 5.4e-02  |
 test_proximal_freeb_compute             |     -0.42 +/- 1.32     | -8.31e-04 +/- 2.63e-03 |  1.98e-01 +/- 1.5e-03  |  1.99e-01 +/- 2.1e-03  |
 test_solve_fixed_iter_compiled          |     +0.31 +/- 0.57     | +6.31e-02 +/- 1.17e-01 |  2.05e+01 +/- 1.0e-01  |  2.04e+01 +/- 5.3e-02  |

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

unalmis added a commit that referenced this pull request Oct 20, 2024
@unalmis unalmis changed the title Writing ε and Γ_c compute funs with new methods ε and Γ_c objectives with Bounce2D Oct 20, 2024
@unalmis unalmis removed the easy Short and simple to code or review label Oct 20, 2024
Copy link

codecov bot commented Dec 19, 2024

Codecov Report

Attention: Patch coverage is 98.71795% with 4 lines in your changes missing coverage. Please review.

Project coverage is 95.64%. Comparing base (e357a9b) to head (08b31b2).
Report is 120 commits behind head on master.

Files with missing lines Patch % Lines
desc/objectives/_fast_ion.py 96.55% 2 Missing ⚠️
desc/equilibrium/coords.py 66.66% 1 Missing ⚠️
desc/objectives/_neoclassical.py 95.45% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1290      +/-   ##
==========================================
+ Coverage   95.61%   95.64%   +0.03%     
==========================================
  Files          98      101       +3     
  Lines       25425    25542     +117     
==========================================
+ Hits        24309    24429     +120     
+ Misses       1116     1113       -3     
Files with missing lines Coverage Δ
desc/batching.py 86.78% <100.00%> (+0.15%) ⬆️
desc/compute/__init__.py 100.00% <ø> (ø)
desc/compute/_deprecated.py 100.00% <100.00%> (ø)
desc/compute/_fast_ion.py 100.00% <100.00%> (ø)
desc/compute/_geometry.py 99.53% <100.00%> (+0.51%) ⬆️
desc/compute/_neoclassical.py 100.00% <100.00%> (+0.91%) ⬆️
desc/grid.py 94.58% <100.00%> (ø)
desc/integrals/_bounce_utils.py 84.61% <100.00%> (+0.24%) ⬆️
desc/integrals/bounce_integral.py 99.48% <100.00%> (+1.54%) ⬆️
desc/integrals/quad_utils.py 100.00% <100.00%> (ø)
... and 10 more

... and 1 file with indirect coverage changes

@unalmis unalmis requested review from a team and removed request for a team December 19, 2024 10:24
@unalmis
Copy link
Collaborator Author

unalmis commented Dec 19, 2024

all requests are now done.

@unalmis unalmis added the P3 Highest Priority, someone is/should be actively working on this label Dec 19, 2024
Copy link
Collaborator

@rahulgaur104 rahulgaur104 left a comment

Choose a reason for hiding this comment

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

A bunch of minor points. I'll ask some more questions over the next hour.

}


def _compute(fun, fun_data, data, grid, num_pitch, simp=False, reduce=True):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does the AD chain go through each iteration of the loop when you do multiple alpha or rho values?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How would I test this

desc/compute/_deprecated.py Show resolved Hide resolved
desc/compute/_deprecated.py Show resolved Hide resolved
desc/compute/_fast_ion.py Show resolved Hide resolved
"max_tz |B|",
"cvdrift0",
"gbdrift (periodic)",
"gbdrift (secular)/phi",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we add a test to ensure that the secular gbdrift gives the same answer as periodic gbdrift as field line length is increased? Otherwise, it can be added later.

Copy link
Collaborator Author

@unalmis unalmis Dec 24, 2024

Choose a reason for hiding this comment

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

It will not because the secular term drives the computation here to zero. We warn this in the objective docstring for now. Without the secular term it converges quickly to basically $\rho * Gamma_c Nemov$

desc/compute/_neoclassical.py Show resolved Hide resolved
desc/integrals/_bounce_utils.py Show resolved Hide resolved
def interp_to_argmin(
h, points, knots, g, dg_dz, method="cubic", beta=-100, upper_sentinel=1e2
):
def interp_to_argmin(h, points, knots, g, dg_dz, method="cubic"):
Copy link
Collaborator

Choose a reason for hiding this comment

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

No suggestion here but can you explain what this function is doing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We interpolate the function h to the global minimum of g that lies between each bounce point. In our application $g = \vert B \vert$ and $h$ is either $\nabla \psi$ or $e_{\alpha}$ at fixed $\rho, \phi$.

"""Test effective ripple with W7-X against NEO."""
eq = get("W7-X")
rho = np.linspace(0, 1, 10)
grid = LinearGrid(rho=rho, M=eq.M_grid, N=eq.N_grid, NFP=eq.NFP, sym=False)
Copy link
Collaborator

Choose a reason for hiding this comment

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

How sensitive is the ripple to the grid resolution now?

@unalmis unalmis merged commit 7d378c2 into master Dec 24, 2024
25 checks passed
@unalmis unalmis deleted the ku/fourier_bounce_neo branch December 24, 2024 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 Highest Priority, someone is/should be actively working on this performance New feature or request to make the code faster stable only merges and reviewer requested changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants