idex_l2a - target/IG fitting routine to be more robust and added fit constraints. #3152
Merged
lacoak21 merged 3 commits intoMay 7, 2026
Merged
Conversation
Collaborator
Author
|
@lacoak21 Would you be available to review this? |
Contributor
|
Hey Alex this looks good. For future PRs can you do the following?
(I did these for you this time) In the actual issue:
|
lacoak21
reviewed
May 7, 2026
Contributor
|
Also the code coverage test is failing because there are if statement paths that are not covered in tests so you will need to add tests for those. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the IDEX L2A waveform-fitting path (estimate_dust_mass / fit_impact) to make baseline estimation and fit parameter constraints more robust, aiming to produce more consistent Target / Ion Grid waveform fits (Issue #3115).
Changes:
- Passes the waveform/channel name into
estimate_dust_massto enable channel-specific fitting constraints. - Reworks baseline estimation and initial parameter guessing for the impact-charge fit.
- Adds bounded optimization to
curve_fitand changes how signal amplitude is computed (analytic extremum vs. discrete max).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+790
to
+792
| window_start = float(np.min(time)) | ||
| window_stop = window_start + 5.0 | ||
| good_mask = np.logical_and(time >= window_start, time <= window_stop) |
Comment on lines
750
to
756
| def estimate_dust_mass( | ||
| low_sampling_time: xr.DataArray, | ||
| target_signal: xr.DataArray, | ||
| remove_noise: bool = True, | ||
| # remove_noise: bool = True, | ||
| remove_noise: bool = False, | ||
| waveform_name: str = "", | ||
| ) -> tuple[NDArray, float, float, float, NDArray]: |
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
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.
Updated idex_l2a.py - fit_impact() to produce more consistent fits.
closes #3115
Change Summary
Updated idex_l2a.py - fit_impact()
Overview
idex_l2a.py
File changes
Made the baseline more robust in fit_impact() and added constraints to the parameters of the fit.
Testing
Re-ran test_idex_l1a through 2b and visually inspected all fits.