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

Effective ripple ε #1003

Merged
merged 231 commits into from
Dec 6, 2024
Merged

Effective ripple ε #1003

merged 231 commits into from
Dec 6, 2024

Conversation

unalmis
Copy link
Collaborator

@unalmis unalmis commented Apr 21, 2024

Resolves #519 . Will be replaced by #1290 soon, so users are advised to wait for that.

Copy link
Contributor

github-actions bot commented Apr 21, 2024

|             benchmark_name             |         dt(%)          |         dt(s)          |        t_new(s)        |        t_old(s)        | 
| -------------------------------------- | ---------------------- | ---------------------- | ---------------------- | ---------------------- |
 test_build_transform_fft_midres         |     +0.41 +/- 4.91     | +2.66e-03 +/- 3.20e-02 |  6.55e-01 +/- 3.1e-02  |  6.53e-01 +/- 9.6e-03  |
 test_build_transform_fft_highres        |     -0.38 +/- 2.33     | -3.88e-03 +/- 2.36e-02 |  1.01e+00 +/- 1.4e-02  |  1.01e+00 +/- 1.9e-02  |
 test_equilibrium_init_lowres            |     +0.83 +/- 1.97     | +3.60e-02 +/- 8.56e-02 |  4.39e+00 +/- 6.2e-02  |  4.35e+00 +/- 5.9e-02  |
 test_objective_compile_atf              |     -0.09 +/- 4.68     | -7.98e-03 +/- 3.99e-01 |  8.53e+00 +/- 3.0e-01  |  8.53e+00 +/- 2.6e-01  |
 test_objective_compute_atf              |     -1.00 +/- 7.03     | -1.17e-04 +/- 8.18e-04 |  1.15e-02 +/- 7.3e-04  |  1.16e-02 +/- 3.7e-04  |
 test_objective_jac_atf                  |     +5.46 +/- 4.21     | +1.07e-01 +/- 8.25e-02 |  2.06e+00 +/- 5.6e-02  |  1.96e+00 +/- 6.1e-02  |
 test_perturb_1                          |     +1.04 +/- 1.71     | +1.63e-01 +/- 2.67e-01 |  1.58e+01 +/- 2.5e-01  |  1.56e+01 +/- 1.0e-01  |
 test_proximal_jac_atf                   |     +0.66 +/- 1.13     | +5.52e-02 +/- 9.43e-02 |  8.42e+00 +/- 7.6e-02  |  8.36e+00 +/- 5.6e-02  |
 test_proximal_freeb_compute             |     +0.29 +/- 1.71     | +5.68e-04 +/- 3.41e-03 |  2.00e-01 +/- 2.0e-03  |  1.99e-01 +/- 2.8e-03  |
 test_solve_fixed_iter_compiled          |     +0.58 +/- 1.40     | +1.03e-01 +/- 2.48e-01 |  1.78e+01 +/- 1.1e-01  |  1.77e+01 +/- 2.2e-01  |
 test_build_transform_fft_lowres         |     -0.17 +/- 5.07     | -9.21e-04 +/- 2.75e-02 |  5.41e-01 +/- 2.1e-02  |  5.42e-01 +/- 1.8e-02  |
 test_equilibrium_init_medres            |     -0.12 +/- 4.07     | -5.04e-03 +/- 1.75e-01 |  4.30e+00 +/- 1.1e-01  |  4.30e+00 +/- 1.3e-01  |
 test_equilibrium_init_highres           |     +0.26 +/- 2.42     | +1.43e-02 +/- 1.35e-01 |  5.60e+00 +/- 1.1e-01  |  5.59e+00 +/- 7.3e-02  |
 test_objective_compile_dshape_current   |     -0.62 +/- 2.02     | -2.45e-02 +/- 7.98e-02 |  3.92e+00 +/- 3.7e-02  |  3.95e+00 +/- 7.1e-02  |
 test_objective_compute_dshape_current   |     +1.12 +/- 2.59     | +4.04e-05 +/- 9.37e-05 |  3.65e-03 +/- 7.5e-05  |  3.61e-03 +/- 5.6e-05  |
 test_objective_jac_dshape_current       |     -2.17 +/- 5.37     | -9.15e-04 +/- 2.27e-03 |  4.13e-02 +/- 9.1e-04  |  4.23e-02 +/- 2.1e-03  |
 test_perturb_2                          |     -0.23 +/- 2.09     | -4.58e-02 +/- 4.13e-01 |  1.97e+01 +/- 2.6e-01  |  1.98e+01 +/- 3.2e-01  |
 test_proximal_freeb_jac                 |     +0.58 +/- 1.98     | +4.33e-02 +/- 1.49e-01 |  7.55e+00 +/- 1.3e-01  |  7.51e+00 +/- 7.5e-02  |
 test_solve_fixed_iter                   |     +1.28 +/- 1.41     | +3.67e-01 +/- 4.03e-01 |  2.90e+01 +/- 2.6e-01  |  2.86e+01 +/- 3.1e-01  |
 test_LinearConstraintProjection_build   |     -0.67 +/- 1.37     | -1.53e-01 +/- 3.13e-01 |  2.27e+01 +/- 2.8e-01  |  2.29e+01 +/- 1.4e-01  |

@unalmis unalmis changed the base branch from master to bounce April 21, 2024 17:06
@unalmis unalmis requested a review from dpanici April 24, 2024 01:43
@gretahibbard
Copy link
Collaborator

Resolves #1288

@unalmis unalmis requested a review from rahulgaur104 December 5, 2024 02:46
@unalmis
Copy link
Collaborator Author

unalmis commented Dec 5, 2024

#1119 and upward is now broken due to git corruption. So let's merge this instead.

@f0uriest
Copy link
Member

f0uriest commented Dec 5, 2024

Can we try to make the objective function api forward compatible with the 2d version?

@unalmis
Copy link
Collaborator Author

unalmis commented Dec 5, 2024

Ok I fixed the issue on the other branch.

@unalmis unalmis removed the stable only merges and reviewer requested changes label Dec 5, 2024
@unalmis
Copy link
Collaborator Author

unalmis commented Dec 5, 2024

Can we try to make the objective function api forward compatible with the 2d version?

I have pushed an update to make it as compatible as possible. The remaining difference is that this objective wants rho and alpha while the 2d methods want an fft compatible (rho, theta, zeta) grid. This difference cannot be made forward compatible.

@unalmis unalmis added the stable only merges and reviewer requested changes label Dec 5, 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.

I have reviewed this once before and used the API enough to be satisfied with this PR.
The discrepancy with WISTELL's ripple calculator is due to significant differences in the numerical methods and I am pretty sure ours are better.
Merge at will!

@unalmis
Copy link
Collaborator Author

unalmis commented Dec 6, 2024

You all have reviewed this multiple times already, and the content of the PR has been split apart and merged via other PRs. At this point this PR is an inoffensive skeleton, and will be replaced by #1290, so I am merging with one approval. No objectives have been added to public api.

@unalmis unalmis merged commit d8d3784 into master Dec 6, 2024
28 checks passed
@unalmis unalmis deleted the ripple branch December 6, 2024 05:30
@YigitElma
Copy link
Collaborator

YigitElma commented Dec 7, 2024

Why the tests of this fail on master? https://github.com/PlasmaControl/DESC/actions/runs/12193541254

EDIT: Oh maybe related to #1434

unalmis added a commit that referenced this pull request Dec 24, 2024
This PR updates the
- `Gamma_c` #1042 
- `effective ripple` #1003

objectives to use the numerical methods from #1119
@unalmis unalmis removed the stable only merges and reviewer requested changes label Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip_changelog No need to update changelog on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add effective ripple
8 participants