Skip to content
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.

Sign up for GitHub

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

Jump to bottom

SweepFormula/psxKernel: Various fixes #2023

Merged
merged 31 commits into from
Dec 20, 2024
Merged

SweepFormula/psxKernel: Various fixes #2023

merged 31 commits into from
Dec 20, 2024

Conversation

t-b
Copy link
Collaborator

@t-b t-b commented Feb 23, 2024 •
edited
Loading

@timjarsky This has our bugfixes for psxKernel and too large decayTau. I've removed the adaptations for the sweep range extraction as #1990 will fix that in a better way (also for old data).

  • Add test
  • Drop empty results in PSX_Operation
  • intersecting ranges bug IVSCCMiniAnalysis on the FTP (same name, new content)
  • StatsRangeTesting: Check that the correct range was used for multi dataset examples
  • Update psxStats documentation
  • Throw away events which have an amplitude which has a different sign compared to the psxKernel amplitude
  • Replace PSX_DEFAULT_X_START_OFFSET with a factor and the decay tau from psxKernel
  • Fix incorrect usage of StatsQuantiles as /Z with err = ... does not make sense
  • psx should return sweepData waves if there are no events, as psxPrep relies on that for the histogram. Otherwise the data psx uses and what psxPrep plots is different
  • Fix kernelAmp sign check again
  • Reverse order of offsetting and filtering, rename all names
  • Hack session with Tim to check that it is now what we want
  • Implement first version with outcome from hack session
  • psx rise time calculation seems to off now (see Sample NWB files from Jessica - playing around.pxp, sweep 48 event 0)
  • Fix bug, Tim uploaded data for assertion (Sample NWB files from Jessica.pxp)
  !!! Assertion FAILED !!!
  Message: "Can not work with NaN as mapIndex"
  Please provide the following information if you contact the MIES developers:
  ################################
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  Stacktrace:
  SF_button_sweepFormula_display(...)#L2995 [MIES_SweepFormula.ipf]
SF_FormulaPlotter(...)#L1920 [MIES_SweepFormula.ipf]
SF_GetTraceColor(...)#L1518 [MIES_SweepFormula.ipf]
SFH_GetLabNoteBookForSweep(...)#L1842 [MIES_SweepFormula_Helpers.ipf]
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  Time: 2024-11-26T14:05:34-08:00
  Locked device: [- none -]
  Current sweep: [- none -]
  DAQ: [- none -]
  Testpulse: [- none -]
  Acquisition state: [- none -]
  Experiment: Sample NWB files from Jessica (Packed)
  Igor Pro version: 9.0.6.1 (56565)
  MIES version:
  
  ################################

Close #2016
Close #2011
Close #1660

@t-b t-b requested a review from timjarsky as a code owner February 23, 2024 14:58
@t-b t-b mentioned this pull request Feb 23, 2024
Closed
1 task
@t-b t-b assigned t-b and unassigned timjarsky Feb 23, 2024
@t-b t-b changed the title SweepFormula/psxKernel: Skip kernel creation for too large decayTau SweepFormula/psxKernel: Various fixes Feb 23, 2024
@t-b t-b assigned timjarsky and unassigned t-b Feb 23, 2024
@timjarsky timjarsky assigned t-b and unassigned timjarsky Feb 23, 2024
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from 718e3ac to 108f961 Compare March 1, 2024 18:24
@t-b t-b assigned timjarsky and unassigned t-b Mar 1, 2024
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from 108f961 to ed6c2fb Compare March 1, 2024 22:19
@timjarsky timjarsky assigned t-b and unassigned timjarsky Mar 7, 2024
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from ed6c2fb to 56305c1 Compare March 8, 2024 18:50
@t-b t-b mentioned this pull request Mar 13, 2024
Closed
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from 56305c1 to 475065e Compare April 18, 2024 19:36
@t-b t-b mentioned this pull request Apr 19, 2024
Merged
@t-b t-b marked this pull request as draft April 19, 2024 18:17
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from 475065e to ffeff20 Compare May 21, 2024 18:15
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from ffeff20 to ffe3676 Compare June 5, 2024 22:13
@t-b t-b assigned timjarsky and unassigned t-b Jun 5, 2024
@t-b

This comment was marked as outdated.

Sign in to view
@t-b t-b marked this pull request as ready for review June 5, 2024 22:13
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from ffe3676 to 7637e16 Compare June 5, 2024 22:38
@t-b t-b mentioned this pull request Jun 6, 2024
Merged
14 tasks
@timjarsky

This comment was marked as outdated.

Sign in to view
t-b added 6 commits December 18, 2024 21:07
@t-b
PSX_UpdateSingleEventGraph: Check passed index
3fcec43 
We can be called from PSX_PlotInteractionHook where we don't know if the
passed index is valid.

Let's do nothing if the index is out of range.
@t-b
SF_PreparePlotter: Remove controls/draw elements on existing panels
eb98115 
In dc9ed87 (SF_PreparePlotter: Don't kill and recreate the main panel
for SF_DM_SUBWINDOWS, 2024-06-19) we switched from killing the
sweepformula plot to clearing it.

But we forgot to remove additional controls and draw elements added by
e.g. PSX.

And now we also have to explicitly remove TraceUserData (TUD) as these are
not cleaned up automatically anymore, as killing an embedded subwindow
does not trigger the main window hook.
@t-b
PSX_ApplyMacroToExistingPanel: Make it static
76bb9fb
@t-b
PSX_ApplyMacroToExistingPanel: Fix execution
450c274 
Due to the changed window handling in dc9ed87 (SF_PreparePlotter: Don't
kill and recreate the main panel for SF_DM_SUBWINDOWS, 2024-06-19) we need
to ensure that the host window of the SF panel is active.
@t-b
DeepCopyWaveRefWave: Fix assertion message
a795222
@t-b
DeepCopyWaveRefWave: Allow src to contain null waves
2b8c2b1 
This makes it easier to use and is more according to our philosopy that null
wave refs should be preferred over empty waves.
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from 1c2e326 to bee7d70 Compare December 18, 2024 21:47
t-b added 7 commits December 18, 2024 23:11
@t-b
DeepCopyWaveRefWave: Nicify the code
db5632b 
We only need index in the else branch and some more spaces are nice.
@t-b
PSX_OperationImpl: Always return a result
61b0cdc 
Using psxPrep requires sweep data from psx. But we used to only return
sweep data if we have found events.

We now always return sweep data so that psxPrep always works.
@t-b
PSX: Change order of offsetting and filtering
ea1bb49 
We now first offset and then filter. This requires that we also rename all
variables, constants and dimension labels.

Change requested by Tim Jarsky.
@t-b
Sweepbrowser: Avoid too old dialog on closing
68b0ff6 
The added reference counting for the sweepbrowser folders in 76cbc80
(AB: Ref count based memory management of loaded DF, 2023-05-10) added a
call to BSP_GetFolder with enabled version checking.

This resulted in a too old panel dialog when trying to close the panel.

Fix it.
@t-b
AB_WindowHook: Save list of files and folders on kill vote
bf6e81b 
This makes closing the panel and reopening it due to an old version
message less annoying.
@t-b
PSX: Add panel version
0a8758f 
In that way we can warn users which have incompatible panels. Same
approach as with other MIES panels.
@t-b
SF_GetTraceColor: Add missing /Z
c695b20
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from bee7d70 to 86fbbbf Compare December 19, 2024 00:26
@timjarsky timjarsky force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from 86fbbbf to affb29e Compare December 19, 2024 00:28
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from affb29e to 0b81d14 Compare December 19, 2024 00:43
t-b and others added 3 commits December 19, 2024 12:39
@t-b
SFH_GetArgumentAsText: Don't try to find unique abbreviations for exa…
b7e3912 
…ct matches

We need to first check for an exact match before we try unique
abbreviations.
@timjarsky @t-b
PSX: Completely overhaul the psx detection algorithm
7418adb 
This overhaul makes it impossible to reuse old data, either from the cache, or the results wave. We do warn
the user about that. And as usual don't delete the old data.

- Gather new values in psxEvent (e.g. Onset Time, Slew Rate and time, ...) and rename entries like post_minXX,
  pre_maxXX, rel_peak, etc.
- Make the calculation clearer and explicitly document where we use the deconvoluated data and where the
  offsetted and filtered sweep data
- Change how we apply the filtering for the deconvoluted and the sweep data
- Make the histogram calculation more robust by using a fixed number of bins
- To make the code easier to grasp we now also calculate the rise time as part of the other event properties
- The extracted single event range now depends on the tau's of all events from that combination
- Introduce more constants for magic numbers
- Add more properties to display for psxstats
- Rework the offsetting in the all events graph and add an entry for slew rate
- Change how we calculate the fit range for the accept average fit
- Enforce that the decay tau is larger than the rise tau for psxKernel
- Add a third parameter to psxRiseTime
- Don't round the peak threshold to three digits as this breaks for very small values
@t-b
Packages/doc/SweepFormula_PSX.rst: Explain the algorithm a bit
9dc4e20
@t-b t-b force-pushed the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch from 0b81d14 to 9dc4e20 Compare December 19, 2024 17:03
@t-b t-b assigned timjarsky and unassigned t-b Dec 19, 2024
@t-b
Copy link
Collaborator Author

t-b commented Dec 19, 2024

@timjarsky Ready!

@t-b t-b enabled auto-merge December 19, 2024 21:38
@t-b
Copy link
Collaborator Author

t-b commented Dec 19, 2024

@timjarsky I've enabled automerge.

@t-b t-b merged commit 643478a into main Dec 20, 2024
20 checks passed
@t-b t-b deleted the bugfix/2023-ignore-too-large-decay-Tau-in-psx-kernel branch December 20, 2024 10:29
@t-b t-b mentioned this pull request Jan 27, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timjarsky timjarsky timjarsky approved these changes

@MichaelHuth MichaelHuth Awaiting requested review from MichaelHuth MichaelHuth is a code owner

Assignees

@timjarsky timjarsky

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make psx more robust psxStats does not support epoch wildcards Allow wildcard expressions in psxStats
2 participants
@t-b @timjarsky