-
Notifications
You must be signed in to change notification settings - Fork 11
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
Improve handling of spectral units #311
Commits on Jul 25, 2024
-
Create "reciprocal_spectroscopy" units context
- Pint tends to choke on conversions between reciprocal energy and wavenumber. These don't come up too often but are an issue when dealing with histograms and spectra. - Here we add a few more "definitions" for Pint to use in such cases. - This creates a slightly scary range of paths around unit conversions. By putting them in a separate context, we can ensure they are only introduced when they are likely to be needed.
Configuration menu - View commit details
-
Copy full SHA for d3b526f - Browse repository at this point
Copy the full SHA d3b526fView commit details
Commits on Jul 26, 2024
-
Use proper units for imported CASTEP DOS
DOS units should be reciprocal of energy/frequency axis: this gives dimensionless area (count) and scales properly with unit changes. It looks like DOS was expressed in enegy/frequency units in part to avoid some Pint conversion challenges. The new reciprocal_spectroscopy context should allow those to be done more safely and directly. There are still some more test failures to be examined and cleaned up.
Configuration menu - View commit details
-
Copy full SHA for 871ae4d - Browse repository at this point
Copy the full SHA 871ae4dView commit details -
Configuration menu - View commit details
-
Copy full SHA for 0f6e69c - Browse repository at this point
Copy the full SHA 0f6e69cView commit details
Commits on Jul 29, 2024
-
Rework broadening internal units handling
- Broadening logic tries to convert to hartree and back again, but this behaves unpredictably (or otherwide badly) for non-energy units. - Instead we can use the bin units; this still requires that the relevant parameters are dimensionally-consistent _with each other_. - While working on this I noticed that very small bin values (i.e. when using large bin _units_) are incorrectly identified as having equal width due to the "atol" which is intended to stabilise comparisons near zero. Bin widths should always be finite, so we can make this stricter and always base agreement on the relative width.
Configuration menu - View commit details
-
Copy full SHA for dfa71d8 - Browse repository at this point
Copy the full SHA dfa71d8View commit details -
Configuration menu - View commit details
-
Copy full SHA for 029e617 - Browse repository at this point
Copy the full SHA 029e617View commit details -
Faster, more robust spectrum getters
The current spectrum getters create a unit conversion factor and then multiply the raw array by it. There are two problems with this: - Multiplying any iterator by a Quantity or Unit leads to Pint checking through all the data for consistency. This can be quite expensive, and is unnecessary for these trusted array objects. When iterating over spectra yielded from a Spectrum1DCollection, the cost can become quite noticeable. - Some unit conversions in the spectroscopy context involve taking a reciprocal, e.g. conversion from cm -> 1/cm is allowed. But if this is done to a _factor_ the data itself will not be inverted. If we start with a Spectrum1D with x_data in wavenumber (1/cm) then spectrum.x_data.to("cm") and spectrum.x_data_unit = "cm" spectrum.x_data will not give the same result; the second method leaves the actual values untouched (as cm / (1/cm) = 1).
Configuration menu - View commit details
-
Copy full SHA for ac02092 - Browse repository at this point
Copy the full SHA ac02092View commit details
Commits on Jul 30, 2024
-
Add full set of length^-1 <-> energy conversions to rs context
By providing Pint with "direct" conversion routes between wavenumber and energy (in each direction and inversion), Pint avoids unnecessarily taking the reciprocal of data and encountering divide-by-zero/infinite-value situations. It doesn't look like the existing divide-by-zero situations were causing problems with correctness of results, but its nice to clear the warnings (and save some CPU?) so that when we see this warning we know it is worth investigating.
Configuration menu - View commit details
-
Copy full SHA for f39424d - Browse repository at this point
Copy the full SHA f39424dView commit details -
Use parens to force reciprocal_spectroscopy application order
Performance tests show this gives a significant improvement for calls to `.to()` and slightly worse performance for (the much-faster) `.ito()`.
Configuration menu - View commit details
-
Copy full SHA for 960ae06 - Browse repository at this point
Copy the full SHA 960ae06View commit details -
Configuration menu - View commit details
-
Copy full SHA for 78961e0 - Browse repository at this point
Copy the full SHA 78961e0View commit details -
Configuration menu - View commit details
-
Copy full SHA for fa0e382 - Browse repository at this point
Copy the full SHA fa0e382View commit details -
Use a non-returning function for unit conversion check
Reviewer found the ``_ =`` distracting, but it is nice to clarify here that we don't care about the return value. Switching to ``.ito()`` also achieves that (and comes with a miniscule performance advantage.)
Configuration menu - View commit details
-
Copy full SHA for 890ac91 - Browse repository at this point
Copy the full SHA 890ac91View commit details -
Use toolz for a cleaner data transformation in test suite
I hope to use toolz more in future, for now it is just needed for this one test!
Configuration menu - View commit details
-
Copy full SHA for 517e8d0 - Browse repository at this point
Copy the full SHA 517e8d0View commit details