Fast X: level populations edition - #466
Open
nabobalis wants to merge 3 commits into
Open
Conversation
Avoid repeated expensive recomputation in the level populations calculation while keeping the output bit-identical: - Cache the Ion instances returned by next_ion()/previous_ion() for internal use so that rate matrices of the recombining ion are only computed once rather than on every density iteration. - Cache the density-independent sum of the two-ion rate matrices. - Memoize plasmapy atomic_number/atomic_symbol/element_name lookups, which construct an expensive Particle object on every call. - Share the proton-electron ratio with the next ion since it only depends on temperature and the abundance/ionization datasets. Benchmark (21 temperatures x 9 densities): Fe XXIII 73.5->7.2 s, Ca XVIII 74.3->8.1 s, O VI 73.3->9.3 s, Fe XII (single-ion model) 9.9->5.0 s, coupled n-T 14.5->3.0 s.
Further speed up of the level populations calculation (and anything else that creates many Ion instances or reads many datasets), again keeping the output bit-identical: - DataIndexerHDF5.__getitem__ now does the membership check, child indexer creation, and read with a single open of the HDF5 file instead of two to three opens per access. - Memoize the set of available ion names and the database fiasco version per database file, keyed on the file modification time and size so that rebuilding the database invalidates the cache. This avoids re-reading the full ion index on every Ion instantiation. Ion instantiation drops from ~3.7 ms to ~1.9 ms and fiasco.proton_electron_ratio from ~2.1 s to ~1.5 s. O VI level populations (21 T x 9 n): 9.3 -> 8.1 s, dominated by np.linalg.solve.
Rather than constructing an Element instance (and thus an Ion instance for every ionization stage) of every element just to read the tabulated ionization fraction data, read the ioneq data directly from the database and instantiate only the first ion of each element to resolve the abundance exactly as before. The iteration order, arithmetic, warning messages, and missing-dataset behavior are unchanged such that the result is bit-identical, including on databases with missing ionization fraction datasets. Also use the memoized atomic number/symbol lookups in list_elements, list_ions, get_isoelectronic_sequence, and Element. fiasco.proton_electron_ratio drops from ~1.4 s to ~0.4 s. O VI level populations (21 T x 9 n): 8.1 -> 6.9 s. The full test suite also runs roughly twice as fast.
Collaborator
Author
|
LOOK AT THE CI FLY. ALSO AS GOOD AS MY PIDGEOT WITH THE HM FLY |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Test to see if Claude MD can speed up some code in this repo.
According to Claude (unverified), these are the speeds up.
Now whether the code is good, is another question.