initializing spectroscopy #196
Conversation
… routines to dataset3deds and dataset3deels
…ataset3dspectroscopy
cophus
left a comment
cophus
left a comment
There was a problem hiding this comment.
Really nice to see the spectroscopy module come together! I went through the data-layout side carefully and the (E, x, y), (scan, scan, energy) refactor is consistent across both EELS and EDS. Energy is the last axis everywhere, including the torch fitting path.
One thing that would make it more robust: the two scan axes get three different names across the module (n_x, n_y / n_y, n_x / w, h), and the EELS class docstring says (scan_y, scan_x, energy). The indexing is correct everywhere, so these aren't bugs, but it makes the code hard to read and easy to break later. Could we standardize on one convention?
Proposed: arrays are (scan_row, scan_col, energy); unpack as scan_row, scan_col, n_energy = self.array.shape; loop indices i_row, i_col; spectra as self.array[i_row, i_col, :].
| self, | ||
| title: str | None = None, | ||
| returnfig: bool = False, | ||
| **kwargs, |
There was a problem hiding this comment.
**kwargs presumably should be passed through to plot?
| from quantem.core.datastructures import Dataset3d as Dataset3d | ||
| from quantem.core.datastructures import Dataset4dstem as Dataset4dstem | ||
| from quantem.spectroscopy import ( | ||
| Dataset3deds as Dataset3deds, |
There was a problem hiding this comment.
Are we agreed on naming it "eds?" I prefer the more explicit name "xeds" i.e. x-ray energy dispersive spectrometry.
| f"expected 4D. Shape: {imported_data['data'].shape}" | ||
| ) | ||
| else: | ||
| # Automatically find first 3D dataset |
There was a problem hiding this comment.
We could add a function to print out the dataset indices, shapes, and other metadata line names if available.
| ) | ||
|
|
||
|
|
||
| class Dataset3deds(Dataset3dspectroscopy): |
There was a problem hiding this comment.
As mentioned above, maybe should be:
Dataset3dxeds
| if return_maps: | ||
| return images, titles | ||
|
|
||
| def Integrate(self, spec, width=0.15, return_maps=False, show=True, **kwargs): |
There was a problem hiding this comment.
I don't think we should support "Integrate" and "integrate." Generally methods should all be lowercase.
| >>> import numpy as np | ||
| >>> from quantem.core.datastructures import Dataset1d | ||
| >>> arr = np.random.rand(10) | ||
| >>> data = Dataset3d.from_array(arr) |
There was a problem hiding this comment.
dataset1d.py:99, 143-146, 186-216, copy-paste docstring errors in the new Dataset1d
| abundance_maps=abundance_maps, | ||
| element_names=list(global_model.peak_model.element_names), | ||
| ) | ||
| if hasattr(self, "_spectrum_images_pytorch"): |
There was a problem hiding this comment.
dataset3deds.py:3431-3441 — dead if/else: the else sets {} then immediately rebuilds from {}, so both branches do the same thing. Collapses to:
base = getattr(self, "_spectrum_images_pytorch", {})
self._spectrum_images_pytorch = {**base, **pytorch_spectrum_images}
| Only returned when *return_maps* is ``True``. | ||
| """ | ||
| if elements is None: | ||
| if self.model_elements is None: |
There was a problem hiding this comment.
dataset3deds.py:487, if self.model_elements is None: never fires (it's initialized to an empty _ModelElementsDict, never None), so the "elements must be specified" guard is dead and users hit a confusing downstream error instead. Use if not self.model_elements: (matches the sibling checks at :2815, :3091).
| return background | ||
|
|
||
|
|
||
| class ExponentialBackground(nn.Module): |
There was a problem hiding this comment.
ExponentialBackground is never used anywhere, and its decay acts on the raw (un-normalized) energy axis unlike PolynomialBackground. Remove it, or wire it up consistently.
| continue | ||
| energy_kev = energy_ev / 1000.0 | ||
|
|
||
| # Use the normalized CSV column as the X-ray line weight. |
There was a problem hiding this comment.
_parse_float(row, ("col4_norm", "weight", ...)) references a col4_norm column that no shipped CSV has; the "normalized CSV column" comment is misleading. Works only by falling through to weight, drop the stale key.
- Bypasses 10-30% window_size constraint in standard subtract_background() - Supports polynomial, linear, and power-law fitting methods - Designed for high-loss only acquisitions and cropped spectra - Useful for O K-edge and other edges with limited pre-edge regions
4 participants
initializing spectroscopy. more details coming soon.