-
Notifications
You must be signed in to change notification settings - Fork 55
Sparse pullback for big performance gain #2170
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
Open
unalmis
wants to merge
114
commits into
master
Choose a base branch
from
ku/sparse_pullback
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
114 commits
Select commit
Hold shift + click to select a range
8fb2b38
Add fft grid and raz grid to test against master
unalmis 25164d0
remove noise by tighten tolerance
unalmis 0d23c66
final attempt
unalmis 5e70567
rory comment
unalmis 945f1af
fix last commit
unalmis c2ecc4b
Increase correlation in discretization error for optimization
unalmis bb8ac6a
Merge branch 'master' into ku/test
unalmis 7489317
.
unalmis e240249
increase tol for test
unalmis be41c58
remove not implemented todo
unalmis a55b170
.
unalmis 2cff860
add back short-circuit
unalmis 1727fba
collect redundant docs
unalmis 339643b
Fix if statements
unalmis bda562a
Merge branch 'master' into ku/test
f0uriest ff53f80
Resolves #2162
unalmis 83ffee6
loosen tol on test
unalmis ccf228f
flake8
unalmis f8a3515
flake8 blank line space
unalmis d05bda1
future proof
unalmis c06687d
daniel comments
unalmis d5a682f
fix render
unalmis f325bbf
Apply suggestions from code review
unalmis 1792e91
Apply suggestions from code review
unalmis 89479dc
Apply suggestions from code review
unalmis 947641b
dan comment v2
unalmis 13b6870
dan v2
unalmis a58c075
more dan
unalmis ad86912
last dan
unalmis f4faed4
last commit to desc
unalmis c42a92b
flake
unalmis 2a5d6c2
Merge branch 'master' into ku/test
unalmis 585d59a
Merge branch 'master' into ku/test
unalmis fcea971
Merge branch 'master' into ku/test
dpanici 86f21f7
Resolves #2168
unalmis 2c93334
remove comment
unalmis 1b79b3e
.
unalmis 1decd5e
clean up internal api
unalmis 6df2ca9
clean
unalmis eef7938
use none
unalmis 7fc978a
remove kwargs over closure conversion
unalmis 8b33ca1
reduce duplicate code
unalmis 5de9a9e
add missing todo
unalmis ae984ca
Remove bounce1d
unalmis ee71551
ad note
unalmis 4da7446
.
unalmis f34abe2
missing exception
unalmis 55155ed
missing label
unalmis b32bc40
Remove kwargs that are not needed anymore
unalmis 1953444
clarify boolean
unalmis 906b26a
.
unalmis f00647c
.
unalmis cd42371
.
unalmis 088d5a2
clarify documentation
unalmis 3fc45c2
fix closure conversion
unalmis 98c9f1b
safer condition for compelx objs
unalmis 566d464
fix pitch_batch_size subtlety
unalmis cdb9bf1
.
unalmis d8ec4c5
.
unalmis 571c7d5
Merge branch 'master' into ku/test
unalmis c5fe484
.
unalmis 7c721b8
Merge branch 'ku/test' into ku/sparse_pullback
unalmis adf73b5
fix comment
unalmis 346938d
Switch resolution to per field period to simplify use and analysis (#…
unalmis e18dfe8
add missing default value
unalmis 8748442
Resolves the fixme comment so that gradients are consistent (#2185)
unalmis d8868fe
push file into zip
unalmis 8676dbd
Resolve remaining comments in #2147
unalmis ac79068
fix param
unalmis fdf80fe
Merge branch 'master' into ku/test
f0uriest 3fe1120
Merge branch 'master' into ku/test
f0uriest bf95c79
Merge branch 'ku/test' into ku/sparse_pullback
unalmis 18234f8
rory stuff
unalmis b1f5da7
rory stuff 2
unalmis 06e22a6
.
unalmis f921c72
rory stuff 3
unalmis 6af9f0f
reuse yb in comment to avoid confusion with nufft eps
unalmis 2de27b0
Merge branch 'master' into ku/test
unalmis 9a365e0
Merge branch 'ku/test' into ku/sparse_pullback
unalmis 9b9d666
Merge branch 'master' into ku/test
unalmis 1002c49
Merge branch 'ku/test' into ku/sparse_pullback
unalmis f729192
Merge branch 'master' into ku/sparse_pullback
unalmis b39ab31
Merge branch 'master' into ku/sparse_pullback
unalmis 1fd2258
Merge branch 'master' into ku/sparse_pullback
unalmis 81cb1a9
address rory
unalmis eaaaff7
Merge branch 'master' into ku/sparse_pullback
unalmis 4955144
Merge branch 'master' into ku/sparse_pullback
unalmis 930569d
@f0uriest
unalmis 2808447
update files
unalmis 95c0e1d
Merge remote-tracking branch 'upstream/master' into ku/sparse_pullback
unalmis d558eae
use external package for sparse pullback
unalmis 99ba3dc
Use external package for sparse support
unalmis 18e0a63
Merge branch 'master' into ku/sparse_pullback
unalmis 3c884e4
Add 'adv-jax-math' to Dependabot configuration
unalmis bc1eab0
.
unalmis becbefa
Merge branch 'master' into ku/sparse_pullback
unalmis 5f0817a
Merge branch 'master' into ku/sparse_pullback
unalmis c8dc88c
merge
unalmis 9a39020
Merge branch 'master' into ku/sparse_pullback
unalmis 897a582
merge
unalmis 3665e13
Merge branch 'master' into ku/sparse_pullback
unalmis 10d6309
Merge
unalmis 7ccaca7
Merge branch 'master' into ku/sparse_pullback
dpanici 951c526
Merge branch 'master' into ku/sparse_pullback
unalmis 20b025e
Merge branch 'master' into ku/sparse_pullback
unalmis 807c13a
Merge branch 'master' into ku/sparse_pullback
unalmis f32aa76
Merge branch 'master' into ku/sparse_pullback
unalmis b5aad57
Merge branch 'master' into ku/sparse_pullback
unalmis 4c32882
Merge branch 'master' into ku/sparse_pullback
dpanici ffa8f24
Merge branch 'master' into ku/sparse_pullback
dpanici a17a8f4
Dp/requested changes sparse (#2291)
dpanici f83456c
fix bug, forgot to change to use keyword for surf_batch_size
dpanici 07418a4
surf_batch_size kwarg bugfix for old 1D computes
dpanici 31c52d3
Merge branch 'master' into ku/sparse_pullback
dpanici File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,9 +4,16 @@ Changelog | |||||||
| Performance Improvements | ||||||||
|
|
||||||||
| - Improves memory management to reduce the base memory used during optimization while using `lsq-exact`, `lsq-auglag` and `fmin-auglag` optimizers. | ||||||||
| - Sparse reverse-mode differentiation was introduced to yield significant performance improvements [#2170](https://github.com/PlasmaControl/DESC/pull/2170). Plumbing to use this method was added that will be progressively taken advantage of in the future. | ||||||||
| - Speeds up ``field_line_integrate`` and ``trace_particles`` for filamentary coils (``Coil``, ``CoilSet``, ``MixedCoilSet``) by precomputing the constant source information, so that the ODE right hand side only evaluates a single fused Biot-Savart kernel instead of recomputing the coil geometry at every solver step. | ||||||||
| - Improves the non-singular Biot-Savart kernel which should give a speed/memory improvement to objectives that compute magnetic field from coils such as ``QuadraticFlux``. | ||||||||
|
|
||||||||
| Breaking Changes and Deprecations | ||||||||
|
|
||||||||
| - The parameter ``num_transit`` in ``EffectiveRipple``, ``Gamma_c``, ``Bounce2D`` and related functions has been changed to ``field_period_transits``. This should make using a consistent resolution across different equilibria easier. The now-deprecated ``num_transit`` may still be used but note the equivalence ``field_period_transits = num_transit * grid.NFP``. | ||||||||
| - The parameter ``Y_B`` in ``EffectiveRipple``, ``Gamma_c``, ``Bounce2D`` is now the resolution over a single field period rather than a full toroidal transit. This should make using a consistent resolution across different equilibria easier. | ||||||||
| - Objectives using ``Bounce2D`` now do not support fwd mode differentiation for JAX versions <0.11.0. | ||||||||
|
|
||||||||
| Bug Fixes | ||||||||
|
|
||||||||
| - Fixes bug in ``auglag`` optimizers which prevented them from accepting solver hyperparameters. | ||||||||
|
|
@@ -16,6 +23,7 @@ Bug Fixes | |||||||
| ``fmin-auglag`` with the default ``tr_method="exact"``, and in | ||||||||
| ``lsq-exact``/``lsq-auglag`` with ``tr_method="cho"``. | ||||||||
|
|
||||||||
|
|
||||||||
| v0.17.3 | ||||||||
| ------- | ||||||||
|
|
||||||||
|
|
@@ -44,11 +52,11 @@ Bug Fixes | |||||||
| - Updates ``"reactor_QA"`` in ``desc.examples`` to fix this. Note that if using ``"reactor_QA"`` example from ``v0.16.0`` until this fix, the current profile in that example has this issue. | ||||||||
| - Fixes bug in `CoilSet.from_symmetry` that ignored the passed in `check_intersection` value. This caused redundant checks in various other functions such as `plot_coils`. | ||||||||
|
|
||||||||
|
|
||||||||
| Breaking Changes | ||||||||
|
|
||||||||
| - Name change in `_CoilObjective` replacing `coilset_mask` with `objective_mask`. Custom subclasses with `_broadcast_input="node"` that previously used `coilset_mask` should switch to `objective_mask`. | ||||||||
|
|
||||||||
|
|
||||||||
| v0.17.2 | ||||||||
| ------- | ||||||||
|
|
||||||||
|
|
@@ -58,7 +66,6 @@ New Features | |||||||
| - Sub-objectives of an `ObjectiveFunction` can now have different `use_jit` values than the `ObjectiveFunction`. These objectives have to be built before building the `ObjectiveFunction`. | ||||||||
| - Adds ``num_neighbors`` parameter to ``CoilSetMinDistance`` that limits the pairwise distance computation to the nearest neighbors per coil, reducing memory useage for large coilsets. | ||||||||
| - Method to plot frequency spectrum of inverse stream map in field line coordinates ``Bounce2D.plot_angle_spectrum``. | ||||||||
| - Method to compute bounce integrals in batches is now added to the public API ``Bounce2D.batch``. | ||||||||
| - Initiated deprecation of ``Bounce2D.compute_fieldline_length`` in favor of ``eq.compute("V_psi")``. | ||||||||
| - The quadrature resolution in ``Bounce2D.compute_fieldline_length`` now corresponds to the resolution over a single field period instead of the resolution over a toroidal transit. | ||||||||
| - Adds an optional attribute `ion_density` to the `Equilibrium` class, to allow the ion density profile to be set independently of the electron density and effective atomic number. Also adds compute functions for ``"ni_rr"`` and ``"Zeff_rr"``. | ||||||||
|
|
@@ -68,6 +75,7 @@ New Features | |||||||
| Bug Fixes | ||||||||
|
|
||||||||
| - Fixes SyntaxError thrown when loading hdf5 data from file-like objects. | ||||||||
| - Fixes ``pitch_batch_size`` argument getting ignored in compute functions. | ||||||||
| - Fixes a bug in `OmnigenousField.change_resolution` when changing `L_B`. | ||||||||
| - Scaling a `ScaledProfile` or taking power of a `PowerProfile` now only updates the `scale`/`power` attributes instead of nesting the `ScaledProfile`/`PowerProfile`s. | ||||||||
| - `jax.Array`s in `_static_attrs` will be automatically converted to `np.ndarray` to prevent stalling code. In general, jax arrays should be omitted in `_static_attrs`. | ||||||||
|
|
@@ -79,7 +87,6 @@ Performance Improvements | |||||||
| - Now, `desc.compute._build_data_index` uses depth-first search algorithm to construct the dependency tree. | ||||||||
| - Some of the default value computations at import time are removed (i.e. `desc.integrals.bounce_integral.default_quad`) | ||||||||
| - [Significantly improves convergence of inverse stream maps](https://github.com/PlasmaControl/DESC/pull/1919). | ||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| - Check-pointing to bounce integrals to improve speed and reduce memory of reverse mode differentiation. | ||||||||
| - Resolves a JAX memory regression in bounce integrals by avoiding materialization of a large tensor in memory. Previously, we had closed the issue by adding nuffts as a workaround. This update actually solves the issue for the case when a user specifies to not use nuffts as well. | ||||||||
| - ``ObjectiveFunction.print_value`` can now use the previously computed ``compute_scaled_error`` values to print. For bounded objectives, we fall back to computing ``compute_unscaled``. Additionally, ``compute_scaled_error`` and array splitting are used in other parts of the code to prevent recompilation for one-time tasks, which makes initialization faster. | ||||||||
|
|
||||||||
|
|
||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.