-
Notifications
You must be signed in to change notification settings - Fork 9
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
Miniature analysis: PSX #1468
Merged
Merged
Miniature analysis: PSX #1468
Conversation
This file contains 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
t-b
force-pushed
the
feature/1468-sweepformula-epsp
branch
2 times, most recently
from
October 19, 2022 12:36
7e9740d
to
cd6b3db
Compare
t-b
force-pushed
the
feature/1468-sweepformula-epsp
branch
from
November 6, 2022 20:29
cd6b3db
to
27eaae6
Compare
t-b
force-pushed
the
feature/1468-sweepformula-epsp
branch
from
November 20, 2022 14:22
27eaae6
to
813b0ad
Compare
t-b
force-pushed
the
feature/1468-sweepformula-epsp
branch
3 times, most recently
from
December 6, 2022 02:27
fa9aaad
to
48ce19a
Compare
t-b
force-pushed
the
feature/1468-sweepformula-epsp
branch
3 times, most recently
from
January 3, 2023 20:36
8f534f3
to
de5a0a6
Compare
t-b
force-pushed
the
feature/1468-sweepformula-epsp
branch
from
January 6, 2023 21:01
de5a0a6
to
9743810
Compare
This comment was marked as outdated.
This comment was marked as outdated.
t-b
force-pushed
the
feature/1468-sweepformula-epsp
branch
2 times, most recently
from
January 12, 2023 21:07
7a2cdc9
to
4ba74f8
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
The operation can now request if it wants markers, lines or any other style. We don't do any error checking with other tweak options, as the developer is responsible for ensuring that all tweaks work together.
This is not a programmer error, so let's be nice.
As this breaks tracing which adds some more lines.
We don't want to use an entry source type for querying as that could conflict with other entries from e.g. store which don't use an entry source type. See also #1825.
The old names taken from the original code where hard to understand. The idea of taking them was that they might contain deeper knowledge. This has, at least for the author of this commit, not happened, so let's name them so that they describe what they contain. This also raises the wave version and includes an upgrade path.
When starting the psx plot with a combination different than zero, the single event fit plot was always not shown. The reason was that the ReplaceWave call in PSX_SetCombo did not find a singleEventFit wave in the new comboDFR and thus did not replace that trace. By creating the wave for all combinations in PSX_MoveWavesToDataFolders we ensure that the wave is always present for all combinations.
Use correct casing for JSON functions and prefer JSON_IsValid.
With making the json document release optional, we can reuse it without having to copy it.
In that way code can already query it before the plotting starts. It is also much shorter code.
We now use 10 * decayTau instead of 30ms (PSX_DEFAULT_DECAY_FIT_LENGTH) as maximum. Requested by Tim Jarsky.
Broken since c7ff079 (SF_FormulaPlotter: Add support for custom markers, 2023-02-27).
It is more natural for the user to enter the number of stdev's from the all points histogram than the peak threshold of the deconvoluated data. This change required to split PSX_OperationImpl in two parts as we need the sweepDataFiltOffDeconv data from all combinations to calculate the peak threshold.
t-b
force-pushed
the
feature/1468-sweepformula-epsp
branch
from
August 4, 2023 12:18
7523826
to
f2344e3
Compare
timjarsky
approved these changes
Aug 4, 2023
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.
Close #1313
Close #1641
Close #1651
Close #1794
Definition of all newly introduced operations:
Done Tasks
Marquee selection, outside of graph ?!, does not work sometimescan't reproduceRemove asssertions from rise time calculation
Restore legend on single event plot not at axis coordinates but absolute positions
Fit is sometimes not shown in single event plot when displaying the first time and restoring the cursor position to event indices > 0
Adapt fitting range to have an upper limit of 10 times the
decayTau
frompsxKernel
instead of the 30msAdd postproc method for psxStats which allows to visualize properties which are not finite. Plot event indices vs. three categories (-inf, NaN, +inf)
psx
topic does not work in the help notebookAdd documentation and tests for new "nonfinite" post processing
Change
peakThreshold
from psx to pass in the number of SDs and calculate peakThreshold from it (see psxPrep), use 2.5 as default (same as psxPrep)Allow cursors/marquee selection in
nonfinite
plot with testsUpdate documentation for removed peakThreshold
Update SweepFormula_PSX.rst and screenshot
All points histogram, Enhance PSX with a way to calculate the peak threshold #1794
Cleanup commits
Think about a way for store to make important data more easily searchable. From slack:Moved to Think about a way for store to make important data more easily searchable #1826Look at documentation again once we are about to merge
Fix "there was an external file system error" Fit result message for failed fit
Proper fix for single event graph not updating
After storing to the results wave, some of the QC data is not restored, Load button bugs out, see external file system error PSX.pxp on FTP
-> different entry source types collide. PSX uses 3 for events and average fit results. But NaN from store does interrupt those when searching. Is it better to use NaN as entrySourceType?!
Fix fit range gathering
Add alignment option using peak instead of onset, add popupmenu to choose, onset is the default
Add
riseTime
option forpsx
, see Miniature analysis: PSX #1468 (comment)Change legend size in single event graph to 100%
Add tests for psxRiseTime
Add tests for riseTime parameter of stats
Fix bug with multiple psxStats with
with
and category axisUpdate SweepFormula.ifn from SweepFormula_WIP.ifn shortly before merge and remove WIP from commit message
PSX preparations #1786
Replace
avg
post processing with multiple return values:mean
,median
,sd
, and anything else that's useful from wavestats;Skewness
Display as category plot, teach SF to allow operations requesting that.store
needs to be able to store the x-labels of the data as well.Update documentation for new behaviour
psxStats with xinterval, see file from Tim, select an event which is failing QC but we requested
acc
psxStats with xinterval needs to recalculate the event intervals as intermediate events with the wrong state should be ignored. Use mean values between both events which means the xinterval as +0.5 is wrong for the general case.
Look into adding a violin plot for post processing. Can those be embedded? Moved into issue.Tim will test cache/results wave handling
PSX_GetStatsResults
: Fix xinterval recalculation for multiple combosheavyManualTestingOfPSX_xinterval_selects_wrong_state.pxp
event 10 of first combo, fit range is offlower stats plot has rejected events but should only have accepted ones. Cursor/Keyboard movement issue?
Missing fits in second combo of same file?!
Investigate why event 0 of combo 0 has amp of NaN
psxStats remove NaN data from
avg
/stats
inputAdd new optional parameter
maxTauFactor
defaulting to 10 to psx, to skip single event fits with taus which are maxTauFactor times larger than the fit rangeFix fit range in PSX_GetEventFitRange
->
n_min_t = i_peak_t + (psxEvent[eventIndex + 1][%i_peak_t] - i_peak_t) * 0.9
and add assertion to ensure that end > start.
Block size ->
Block size [%]
Don't allow keyboard interaction in all event graph when
fit state
is selected, warn userNeeds to wait for Switch to JSON wave serialization for store in all cases #1731 as that touches SweepFormula.ifn
Adapt to changes in Switch to JSON wave serialization for store in all cases #1731
Look into psx_ALLqcDisappeared.pxp, why is data from the results wave not displayed?
psx to use the cache directly. Store its result with a key which is derived from the combination and all psx/psxEvent parameters
Add Load button to load from results wave
Restore from results on load if there is no entry in the cache
Use derived key (combination and psx/psxEvent parameters) as results and cache key
Use comboKey and psxParameters (including the name) for deriving the cache key only
psx/psxStats will gain name parameter, strict igor object name, to identify dataset when storing/loading from/to the results wave
psx operation is using the cache to improve performance and also to store QC from the user
psxStats will use all the data which either psx has under that name, or what it can gather from the results wave
psx only supports the same name with
with
,and
is not supported.psxstats will keep the selectData parameter for subselection of the named data. Bail out on conflict.
This supports the following three uses cases:
User enters psx formula, does QC, changes psx paramaters , does QC, and now goes back to the first parameters set. Her QC results are still there because of the cache
User wants to permanently store the QC results into the results wave using a name
The results wave entry from the previous example can anytime be overwritten with new paramters, If the user then tries to load data from results wave that the name is correct for psx but the parameters are different we don't load the data.
Prevent hiding controls in the bar on the left when resizing the QC window -> Use Resize Controls
Use dark grey for single event fit trace
Investigate:
| Sometimes the fit trace appears to be missing from the plot, even though there is a fit result. We may need to do a screen share to determine the cause.
Make tests passing again
PSX_UpdateSingleEventGraph must not use CurrentCombo but the combo of the selected event. Happens when selecting events from stats plot.
Rebase against main and run tests
Average Fit: Rename KX to meaningful names
Fix mouse selection with log
Preserve shown info on sweepformula in general
Average Fit does often not converge. Supply initial guesses from our known time constants (average taus, psxKernel input) to make it more robust. Or maybe fit separately with two exponentials (split at min or max) and use those results as initial values for the double exponential fit.
All event plot: Add SetVariable for
%
events of current combo/all combos to show only a subset of events (aka one block), popup menu to select which block to showAdd tests for block index logic
Can/should we allow multiple sweepbrowsers using psx to work on the same data? How can we avoid that those interact badly? -> Don't store event data explicitly and implicitly for duplicated sweepbrowser.
Investigate failing fit from fit_issue.pxp (
E:\tim-data\issue-1313
)Allow fitting function for accepted fitting to be choosen: Either dblexp_peak or dblexp_XOffset Default to dblexp_peak.
Rename tau1/tau2 to riseTau/decayTau for psxKernel
Q: Would it be possible to plot Xinterval against event no. by giving the last event a NaN interval? This would make the plotting of Xinterval useful, especially in the case of the histogram.
Q: I'm also seeing an issue where
all' and 'every
fails to plot - I'll upload an example pxp to your FTP later today. In these cases,accepted
works as expected.Q: If the
store
function is used with psxStats...hist, I believe it stores the counts in each bin but not the x value of the bin. What's the recommended way to access the x info?A: Change
store
format to also use JSON serialization which should take care of dimdelta/offset. Adapt our other internal users ofstore
.Use average of event start time, calculate also the average of the event lengths and use that as fit length.
Fix initial size of SF panel/graph
Add rst documentation for GUI
(right before the merge) Add notebook documentation for operations
Think about displaying the mouse/keyboard interaction options somewhere
Revise rst documentation
Add some more tests:
Add tests for
epspKernel()
Add documentation for
epspKernel()
Add tests for
epsp()
Add documentation for
epsp()
Add documentation for
epspStats()
Add tests for mouse event selection
All events plot: Maintain axes scaling after the user has modified scaling in
ES_MouseSelection with multiple ranges does not yet work
ES_MouseSelection support log10 post processing and logX axis scaling
Don't use a button for the 'i' info
determine the action and we act on the clicked trace. See
TraceFromPixel
.Add PSX: Allow jumping to selected events in epspStats plot #1641
Allow cursor A in epspStats plot to select the current event (ignore other cursors), update single event view and cursor position in epsp plot, change combination as well
All events plot:
Go back to button for i, copy to clipboard on press
allow shortest unique abbbreviation for text parameters
using arrow keys to jump between ranges when reviewing events, when at the end of the begining go to the next/previous range/combination
jump to the first event when changing the range/combiation
allow combinations without peaks
zerowave before filtering
State should be associated with the prop:
amp/xpos/xinterval/estate -> event state
tau/fstate/fitresult -> fit state
Chat 20.3:
Deconv end point handling for filtering is not yet optimalLook into example data on FTP which shows ringing in deconv trace (every 20/40ms)-> PSX: Improve deconv filtering #1662
epspStats
handles multiple combinations where one of them does not have events (none detected byepsp
)[E0, E1]
for stats, create issue to handle wildcardE*