PSX iterations #2427
Open
PSX iterations #2427
Conversation
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
The returned wave is the same as was passed in. Although the current code makes it clear what is input and what is output, this creates verbose code as you can't have the same wave as input (RHS) and output (LHS) of an equation. So let's drop the returned wave.
And rename them to emphasize the label component as the y data unit is only the fallback.
In e26f03a (SF: Adapt logic how x-waves are mapped to y-waves, 2022-08-19) we added code to special case x/y wave mapping. But we don't need to do that if we have single point x and y waves only.
This preserves wave metadata like units.
…s' into HEAD * bugfix/2423-sweepformula-multiple-axis-labels-for-x-axis: SF_GatherAxisLabels: Also gather the x-labels SF_FormatUnit: Factor it out SF_GatherFormulaResults: Prefer Duplicate over new Make SF_GatherFormulaResults: Skip special case for single x and y wave pairs Tests: Add TestAxisLabelGathering SF_FormulaPlotter: Move SF_CombineAxisLabels closer to its usage SF_FormulaPlotter: Generalize y axis label handling SF_GatherYUnits: Drop return value SF_CombineYUnits: Prefer TextWaveToList SF_FormulaPlotter: Factor traceCnt check out
By accepting the text wave to be pretty printed we can let the visualized data and the data copied to the clipboard differ so that the latter can be more easily C&P'ed into tab calculations like Excel/Calc.
…nto HEAD * feature/2424-easier-copy-and-paste-from-psx-to-excel: UpdateInfoButtonHelp: Set machine-readable text for C&P
We already call PSX_UpdateAllEventGraph with all flags set in PSX_PostPlot. So using PGC for each control and doing a partial update is just a waste of CPU cycles. This also fixes a bug where the Fit accepted average checkbox was checked and the restore triggered an assertion as the single event waves were not yet up-to-date.
When having a SF formula like psxStats vs [0, 1] the xWave has not the same size anymore. So we need to refetch the x-wave in this case.
Taking the start of the single event (0) does not work that well. So let's take 10% of the mean peak time instead.
Requested by Tim Jarsky.
The current algorithm for determining the left boundary for the baseline search did not take into account events which are close-by. Fix that by using the peak time of the previous event as maximum boundary.
We do override the box size for the find peak search as that is a quick fix but not that dirty. Some minor test adjustments were also required.
When the calculated average standard deviation is NaN we can still get a tau of NaN. Use the fallback value in this case.
And rename the original function to PSX_CalculateRiseTimeWrapper.
This is always zero.
…tion for accepted average fit
…al fit For the individual event decay fit. Weighted tau is computed for each event.
…events average wave
New MiES algorithmic utility for band pass filtering with Nan, Inf, and rinnging rejection and automatic order reduction
Rename it to psxDeconvBPFilter to make it clear that this is a bandpass filter, see also PSX_DeconvoluteSweepData. And also reorder the filter frequencies for the user to avoid an assertion.
Due to implementation details of how FilterIIR implements band pass filters the used order is always even. If required, odd values are increased by one so that they are even. Document that and also make them even already when we get them. We can as well adap the default value to be even already.
Define all variables at the top, reword the documentation.
When PSX_OperationSweepGathering returns 1 as error code, we wrote the code so that idx is not incremented in this case. The idea in d359a7a (PSX_Operation: Handle partial results better, 2024-02-27) was that we don't want holes in the output wave. But the code was buggy as it, after an error, would reuse the same data again, and thus error out again, and therefore ignoring all data behind the first combination with an error. Fix that by introducing separate indizes for reading and writing.
A common tweaking algorithm done by users is to find the lowest filter order which has the data from all combinations passing. As that is manually quite tedious we now do that for her. The idea is to start at some maximum order and then get he realized order for each combination. If the realized order is constant for all combinations, we are done. Otherwise we try the minimum order again and should then be finished. This is done for both sweep and deconv.
This gives better error messages.
We need to preprocess the code before we can extract the variables and the formula. Broken since 8ec6945 (SF: Add support to check also variables with the check button, 2023-03-29).
There was a problem hiding this comment.
Pull Request Overview
This PR implements PSX iterations functionality and generalizes the deconvolution filter for all four filter types with a new name. Key changes include:
- Refactored PSX filtering to use a new generalized BandPassWithRingingDetection function
- Renamed psxDeconvFilter to psxDeconvBPFilter and added psxSweepBPFilter for comprehensive filter management
- Enhanced PSX event data structure to support double exponential fitting with separate fast/slow/weighted tau values
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Packages/tests/Basic/UTF_SweepFormula_PSX.ipf | Updated test cases for new filter naming and expanded event data structure |
| Packages/tests/Basic/UTF_SweepFormula.ipf | Added test cases for axis label gathering and input code validation |
| Packages/doc/SweepFormula.rst | Updated documentation to reflect psxDeconvBPFilter name change |
| Packages/MIES/MIES_WaveDataFolderGetters.ipf | Expanded PSX event wave structure and added new data folder functions |
| Packages/MIES/MIES_Utilities_GUI.ipf | Modified button help system to accept wave input instead of string |
| Packages/MIES/MIES_SweepFormula_PSX_Macro.ipf | Updated GUI panel layout with new visualization controls |
| Packages/MIES/MIES_SweepFormula_PSX.ipf | Major refactoring of PSX analysis with new filtering approach and expanded event fitting |
| Packages/MIES/MIES_SweepFormula_Helpers.ipf | Added assertion for parameter validation |
| Packages/MIES/MIES_SweepFormula.ipf | Updated operation registry and axis label handling |
| Packages/MIES/MIES_MiesUtilities_Algorithm.ipf | Added BandPassWithRingingDetection function |
| Packages/MIES/MIES_Constants.ipf | Updated constants for new filter operations and error codes |
| Packages/MIES/MIES_Cache.ipf | Updated cache version numbers for PSX operations |
Comments suppressed due to low confidence (1)
| WaveStats/M=1/Q/R=(baseline_t - range, baseline_t + range) sweepDataOffFilt | ||
| baseline = V_avg | ||
|
|
||
| ASSERT(isFinite(baseline), "basline is not finite") |
There was a problem hiding this comment.
Spelling error in assertion message: 'basline' should be 'baseline'.
Suggested change
| ASSERT(isFinite(baseline), "basline is not finite") | |
| ASSERT(isFinite(baseline), "baseline is not finite") |
| kernelDecayTau = JWN_GetNumberFromWaveNote(psxKernelDataset, path + "/decayTau") | ||
| ASSERT(IsFinite(kernelDecayTau), "decayTau must be finite") | ||
| kernelIterations = JWN_GetNumberFromWaveNote(psxKernelDataset, path + "/iterations") | ||
| ASSERT(IsInteger(kernelIterations), "decayTau must be an integer") |
There was a problem hiding this comment.
Error message refers to 'decayTau' but should refer to 'kernelIterations' to match the variable being checked.
Suggested change
| ASSERT(IsInteger(kernelIterations), "decayTau must be an integer") | |
| ASSERT(IsInteger(kernelIterations), "kernelIterations must be an integer") |
|
|
||
| // TODO id | ||
| Make/FREE/D results = {kernelAmp, kernelRiseTau, kernelDecayTau, kernelIterations} | ||
| SetDimensionLabels(results, "id;kernelRiseTau;kernelDecayTau;kernelAmp;kernelIterations", ROWS) |
There was a problem hiding this comment.
The dimension labels don't match the data in the results wave. The first element is kernelAmp, but the label is 'id'. The order should be corrected to match the actual data assignment: kernelAmp, kernelRiseTau, kernelDecayTau, kernelIterations.
Suggested change
| SetDimensionLabels(results, "id;kernelRiseTau;kernelDecayTau;kernelAmp;kernelIterations", ROWS) | |
| SetDimensionLabels(results, "kernelAmp;kernelRiseTau;kernelDecayTau;kernelIterations", ROWS) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
psxKernela pure data containerpsxAddPsxIterations([iterations = 0])operationiterations > 0