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 3 commits into
base: master
Choose a base branch
from
Open

Adding old objectives #1514

wants to merge 3 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         |     -2.81 +/- 6.91     | -1.67e-02 +/- 4.10e-02 |  5.77e-01 +/- 3.6e-02  |  5.94e-01 +/- 1.9e-02  |
 test_equilibrium_init_medres            |     -1.91 +/- 4.60     | -8.92e-02 +/- 2.14e-01 |  4.58e+00 +/- 1.6e-01  |  4.67e+00 +/- 1.4e-01  |
 test_equilibrium_init_highres           |     +1.52 +/- 2.02     | +8.75e-02 +/- 1.16e-01 |  5.82e+00 +/- 8.5e-02  |  5.74e+00 +/- 7.9e-02  |
 test_objective_compile_dshape_current   |     -0.66 +/- 2.07     | -2.68e-02 +/- 8.46e-02 |  4.07e+00 +/- 6.0e-02  |  4.09e+00 +/- 6.0e-02  |
 test_objective_compute_dshape_current   |     +8.49 +/- 12.56    | +4.44e-04 +/- 6.57e-04 |  5.67e-03 +/- 6.5e-04  |  5.23e-03 +/- 9.3e-05  |
 test_objective_jac_dshape_current       |     +4.08 +/- 5.32     | +1.74e-03 +/- 2.27e-03 |  4.45e-02 +/- 1.2e-03  |  4.27e-02 +/- 1.9e-03  |
 test_perturb_2                          |     +2.61 +/- 2.67     | +5.49e-01 +/- 5.62e-01 |  2.16e+01 +/- 5.5e-01  |  2.10e+01 +/- 1.1e-01  |
 test_proximal_freeb_jac                 |     +1.65 +/- 3.34     | +1.24e-01 +/- 2.52e-01 |  7.67e+00 +/- 1.5e-01  |  7.55e+00 +/- 2.0e-01  |
 test_solve_fixed_iter                   |     +0.87 +/- 2.89     | +2.90e-01 +/- 9.68e-01 |  3.38e+01 +/- 5.9e-01  |  3.35e+01 +/- 7.7e-01  |
 test_LinearConstraintProjection_build   |     +2.78 +/- 3.56     | +3.02e-01 +/- 3.87e-01 |  1.12e+01 +/- 2.7e-01  |  1.09e+01 +/- 2.8e-01  |
 test_build_transform_fft_midres         |     +0.72 +/- 4.22     | +4.39e-03 +/- 2.56e-02 |  6.11e-01 +/- 2.5e-02  |  6.06e-01 +/- 6.2e-03  |
 test_build_transform_fft_highres        |     -0.34 +/- 1.26     | -3.33e-03 +/- 1.23e-02 |  9.72e-01 +/- 8.6e-03  |  9.75e-01 +/- 8.8e-03  |
 test_equilibrium_init_lowres            |     +1.07 +/- 1.27     | +4.11e-02 +/- 4.85e-02 |  3.87e+00 +/- 2.7e-02  |  3.82e+00 +/- 4.1e-02  |
 test_objective_compile_atf              |     +0.39 +/- 1.81     | +3.23e-02 +/- 1.48e-01 |  8.22e+00 +/- 1.1e-01  |  8.19e+00 +/- 9.9e-02  |
 test_objective_compute_atf              |     +0.86 +/- 1.48     | +1.35e-04 +/- 2.34e-04 |  1.59e-02 +/- 1.6e-04  |  1.58e-02 +/- 1.7e-04  |
 test_objective_jac_atf                  |     -1.09 +/- 2.40     | -2.19e-02 +/- 4.80e-02 |  1.98e+00 +/- 3.4e-02  |  2.00e+00 +/- 3.4e-02  |
 test_perturb_1                          |     +0.75 +/- 1.02     | +1.09e-01 +/- 1.49e-01 |  1.47e+01 +/- 1.3e-01  |  1.46e+01 +/- 7.7e-02  |
 test_proximal_jac_atf                   |     +0.34 +/- 0.89     | +2.81e-02 +/- 7.44e-02 |  8.35e+00 +/- 6.3e-02  |  8.32e+00 +/- 4.0e-02  |
 test_proximal_freeb_compute             |     +1.67 +/- 0.88     | +3.31e-03 +/- 1.73e-03 |  2.01e-01 +/- 1.2e-03  |  1.98e-01 +/- 1.2e-03  |
 test_solve_fixed_iter_compiled          |     +0.12 +/- 0.73     | +2.55e-02 +/- 1.52e-01 |  2.07e+01 +/- 1.3e-01  |  2.07e+01 +/- 8.6e-02  |

Copy link

codecov bot commented Jan 9, 2025

Codecov Report

Attention: Patch coverage is 91.78082% with 6 lines in your changes missing coverage. Please review.

Project coverage is 95.67%. Comparing base (e2fd5fb) to head (82be7f5).
Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
desc/objectives/_fast_ion.py 91.66% 3 Missing ⚠️
desc/objectives/_neoclassical.py 91.42% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1514      +/-   ##
==========================================
- Coverage   95.68%   95.67%   -0.02%     
==========================================
  Files         101      101              
  Lines       25566    25637      +71     
==========================================
+ Hits        24464    24529      +65     
- Misses       1102     1108       +6     
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/bounce_integral.py 97.10% <ø> (ø)
desc/objectives/__init__.py 100.00% <100.00%> (ø)
desc/objectives/_fast_ion.py 95.74% <91.66%> (-2.54%) ⬇️
desc/objectives/_neoclassical.py 95.65% <91.42%> (-2.60%) ⬇️

... and 2 files with indirect coverage changes

@dpanici
Copy link
Collaborator

dpanici commented Jan 15, 2025

Check with an older JAX version @dpanici

@@ -283,3 +283,120 @@ def compute(self, params, constants=None):
**self._hyperparam,
)
return constants["transforms"]["grid"].compress(data["effective ripple"])


class OldEffectiveRipple(EffectiveRipple): # 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.

maybe keep it a single class but add a interpolation_method flag to be 1D or 2D

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.

2 participants