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

Adding old objectives #1514

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Adding old objectives #1514

wants to merge 6 commits into from

Conversation

unalmis
Copy link
Collaborator

@unalmis unalmis commented Jan 9, 2025

until the nan in forward for GammaC is resolved (1489) and nuffts are used (#1294)

…e made:

These are the same ones documented in the docstring of the new methods.
    Performance will improve significantly by resolving these GitHub issues.
      * ``1154`` Improve coordinate mapping performance
      * ``1294`` Nonuniform fast transforms
      * ``1303`` Patch for differentiable code with dynamic shapes
      * ``1206`` Upsample data above midplane to full grid assuming stellarator symmetry
      * ``1034`` Optimizers/objectives with auxiliary output
@unalmis unalmis added stable only merges and reviewer requested changes P1 Lowest Priority, will get to eventually skip_changelog No need to update changelog on this PR labels Jan 9, 2025
Copy link
Contributor

github-actions bot commented Jan 9, 2025

|             benchmark_name             |         dt(%)          |         dt(s)          |        t_new(s)        |        t_old(s)        | 
| -------------------------------------- | ---------------------- | ---------------------- | ---------------------- | ---------------------- |
 test_build_transform_fft_lowres         |     -7.38 +/- 18.20    | -4.58e-02 +/- 1.13e-01 |  5.74e-01 +/- 4.5e-02  |  6.20e-01 +/- 1.0e-01  |
 test_equilibrium_init_medres            |     +1.08 +/- 5.54     | +4.62e-02 +/- 2.36e-01 |  4.31e+00 +/- 2.2e-01  |  4.26e+00 +/- 9.7e-02  |
 test_equilibrium_init_highres           |     +0.25 +/- 5.37     | +1.42e-02 +/- 3.01e-01 |  5.61e+00 +/- 1.5e-01  |  5.60e+00 +/- 2.6e-01  |
 test_objective_compile_dshape_current   |     -1.09 +/- 6.98     | -4.67e-02 +/- 2.99e-01 |  4.24e+00 +/- 1.9e-01  |  4.28e+00 +/- 2.3e-01  |
 test_objective_compute_dshape_current   |     +0.54 +/- 1.49     | +2.80e-05 +/- 7.74e-05 |  5.22e-03 +/- 6.3e-05  |  5.19e-03 +/- 4.4e-05  |
 test_objective_jac_dshape_current       |     -2.15 +/- 14.27    | -9.82e-04 +/- 6.51e-03 |  4.47e-02 +/- 2.8e-03  |  4.57e-02 +/- 5.9e-03  |
 test_perturb_2                          |     +2.38 +/- 3.64     | +4.84e-01 +/- 7.43e-01 |  2.09e+01 +/- 5.3e-01  |  2.04e+01 +/- 5.2e-01  |
 test_proximal_freeb_jac                 |     -0.25 +/- 3.59     | -1.87e-02 +/- 2.72e-01 |  7.56e+00 +/- 2.4e-01  |  7.58e+00 +/- 1.3e-01  |
 test_solve_fixed_iter                   |     -1.23 +/- 2.06     | -4.17e-01 +/- 6.96e-01 |  3.34e+01 +/- 5.1e-01  |  3.38e+01 +/- 4.7e-01  |
 test_LinearConstraintProjection_build   |     -0.32 +/- 3.70     | -3.46e-02 +/- 3.98e-01 |  1.07e+01 +/- 1.9e-01  |  1.08e+01 +/- 3.5e-01  |
 test_build_transform_fft_midres         |     +2.38 +/- 7.73     | +1.46e-02 +/- 4.76e-02 |  6.30e-01 +/- 4.6e-02  |  6.15e-01 +/- 1.0e-02  |
 test_build_transform_fft_highres        |     -1.20 +/- 3.31     | -1.18e-02 +/- 3.26e-02 |  9.73e-01 +/- 2.9e-02  |  9.85e-01 +/- 1.4e-02  |
 test_equilibrium_init_lowres            |     +0.16 +/- 2.96     | +6.30e-03 +/- 1.17e-01 |  3.95e+00 +/- 1.1e-01  |  3.94e+00 +/- 3.5e-02  |
 test_objective_compile_atf              |     -0.79 +/- 1.35     | -6.59e-02 +/- 1.13e-01 |  8.29e+00 +/- 9.9e-02  |  8.36e+00 +/- 5.3e-02  |
 test_objective_compute_atf              |     -0.75 +/- 1.79     | -1.19e-04 +/- 2.86e-04 |  1.58e-02 +/- 1.6e-04  |  1.59e-02 +/- 2.4e-04  |
 test_objective_jac_atf                  |     -0.59 +/- 2.44     | -1.15e-02 +/- 4.82e-02 |  1.96e+00 +/- 2.9e-02  |  1.97e+00 +/- 3.8e-02  |
 test_perturb_1                          |     +0.04 +/- 1.53     | +6.53e-03 +/- 2.26e-01 |  1.48e+01 +/- 1.7e-01  |  1.48e+01 +/- 1.5e-01  |
 test_proximal_jac_atf                   |     +0.27 +/- 0.86     | +2.26e-02 +/- 7.12e-02 |  8.29e+00 +/- 3.9e-02  |  8.27e+00 +/- 5.9e-02  |
 test_proximal_freeb_compute             |     +0.22 +/- 1.76     | +4.35e-04 +/- 3.54e-03 |  2.01e-01 +/- 2.6e-03  |  2.01e-01 +/- 2.4e-03  |
 test_solve_fixed_iter_compiled          |     -0.09 +/- 0.77     | -1.80e-02 +/- 1.59e-01 |  2.07e+01 +/- 1.3e-01  |  2.07e+01 +/- 8.5e-02  |

Copy link

codecov bot commented Jan 9, 2025

Codecov Report

Attention: Patch coverage is 96.61017% with 2 lines in your changes missing coverage. Please review.

Project coverage is 95.69%. Comparing base (b3b6dcc) to head (a2fd573).

Files with missing lines Patch % Lines
desc/objectives/_fast_ion.py 96.55% 1 Missing ⚠️
desc/objectives/_neoclassical.py 96.42% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1514   +/-   ##
=======================================
  Coverage   95.69%   95.69%           
=======================================
  Files         101      101           
  Lines       25614    25671   +57     
=======================================
+ Hits        24511    24567   +56     
- Misses       1103     1104    +1     
Files with missing lines Coverage Δ
desc/compute/__init__.py 100.00% <ø> (ø)
desc/compute/_neoclassical.py 100.00% <ø> (ø)
desc/compute/_old.py 100.00% <ø> (ø)
desc/integrals/_interp_utils.py 100.00% <ø> (ø)
desc/integrals/bounce_integral.py 97.10% <ø> (ø)
desc/objectives/__init__.py 100.00% <100.00%> (ø)
desc/objectives/_fast_ion.py 97.70% <96.55%> (-0.58%) ⬇️
desc/objectives/_neoclassical.py 97.64% <96.42%> (-0.60%) ⬇️

... and 1 file with indirect coverage changes

@dpanici
Copy link
Collaborator

dpanici commented Jan 15, 2025

Check with an older JAX version @dpanici

@unalmis unalmis self-assigned this Jan 26, 2025
@unalmis unalmis requested review from a team, rahulgaur104, f0uriest, ddudt, dpanici, sinaatalay and YigitElma and removed request for a team January 29, 2025 00:46
@@ -295,3 +303,105 @@ def compute(self, params, constants=None):
**self._hyperparam,
)
return constants["transforms"]["grid"].compress(data[self._key])


class GammaC_Spline(GammaC): # noqa: D101
Copy link
Collaborator

Choose a reason for hiding this comment

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

use a flag to choose method

@f0uriest
Copy link
Member

This would also be a good place to add benchmarks for the bounce integral objectives (compute and jacobian, ideally using both 1d and 2d versions)

Copy link
Member

Choose a reason for hiding this comment

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

maybe something like _neoclassical_1d.py rather than just old or deprecated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Lowest Priority, will get to eventually skip_changelog No need to update changelog on this PR stable only merges and reviewer requested changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants