MIES icon indicating copy to clipboard operation
MIES copied to clipboard

Various PSX fixes

Open t-b opened this issue 10 months ago • 20 comments

  • [x] FFT size fix using 2^l + 3^k logic, closes: #1663
  • [x] psx accepted average fix: return real amplitude for double expontential
  • [x] average fit for reject, undet, all, add into column on the right
  • [x] Add default values for filter frequencies:
sweepFilter: Default values for low/high, order defaults to 4
low cutoff  = 1 / (3 * pi * riseTau)
high cutoff = 1 / (3 * pi * decayTau)
deconvFilter: Default values for low/high, order defaults to 4
low cutoff  = 1 / (4 * pi * riseTau)
high cutoff = 1 / (4 * pi * decayTau)
  • [x] Add y value from PSX_CalculateOnsetTime into psxEvent and psxstats
  • [x] add the fit results as a wave in psx
  • [x] the "block size" control doesn't respect the "current combo" checkbox state
  • [x] add peakThreshold in () behind numberOfSDs in the JWN to document it
  • [x] Add V_mode and all other missing entries from StatsQuantiles to postProc output for stats in psxStats
  • [x] Prep PR: https://github.com/AllenInstitute/MIES/pull/2471
  • [x] TB: Dig up store/results code from slack
  • [x] ~~Tim provides Poc for figuring out psxKernel parameters from scratch~~
  • [x] Approval from Tim
  • [x] Fix conflicts
  • [ ] Fix commits a bit
  • [ ] Fix tests
  • [ ] Update ihf documentation
Old Todos

So this should be ready for a first test. I've adapted `psxKernel` and `psxStats` to allow passing in multiple selections.

  • [x] Fix assertion, https://aiephys.slack.com/archives/C06P3MYEV6H/p1742330027060869
  • [x] https://github.com/AllenInstitute/MIES/pull/2376
  • [x] Yesterday, we chatted about moving the baseline search before peak_t, and I think that still makes sense. The baseline search shouldn't search past the previous peak_t. With the baseline_t of the event available, PSX_CalculateEventPeak, instead of using prevDeconvPeak_t to constrain the search, can use the event baseline_tinstead. The above makes adding a peakfinding operation, which I suggested yesterday, unnecessary.
  • [x] Tweak accept fit average logic: The start point for the fit should be calculated via rise time calculation (20% of amplitude, add control "fit start amplitude"). Use that for onset and peak methods.
  • [x] Debug session with Tim
  • [x] Polish code
    • [x] // TODO add vertical dashed lines at positions // 3 checkboxes to turn all of these on/off below suppress update // peak_t // baseline_t
    • [x] Fix assertion when restoring PSX panel with accepted fit checkbox checked
  • [x] Add three taus to stats
  • [x] Wait for Tim's changes
  • [x] Fix function comment PSX_AppendTracesToAllEventGraph
  • [x] https://aiephys.slack.com/archives/D6WGZV2V8/p1743629846161639
  • [x] Fix slow/fast tau
  • [x] Excel import could be better, see https://github.com/AllenInstitute/MIES/pull/2424
  • [x] Commit cleanup
  • [x] Add psxBPSweepFilter (BP = bandpass) and back down order for sweep/deconv. Handle the high/low frequency input arguments for the user such that the filtering is always bandpass.
  • [x] Rename psxDeconFilter to include the filter type acronym (is this also a bandpass filter?). Manage the frequency arguments for the user such that it doesn't matter what order they are provided.
# psxSweepFilter(lowFreq, highFreq [, order = 6])
# back down on having nonfinite values in filtered data, order -2, output passing order
# back down on deconv order as well, same approach
# use code from Tim to determine the highest possible, reapply that order to all sweep data
  • [x] Output all x/y axis labels for multiple operations with with, see https://github.com/AllenInstitute/MIES/pull/2423
  • [x] Later: Decide later on about support for multiple properties in psxStats, see https://github.com/AllenInstitute/MIES/issues/2422
  • [x] #2423
  • [x] #2424

t-b avatar Feb 28 '25 17:02 t-b

@t-b The deconvolved trace appears to be shorter in time/length that the range trace image

I'm uploading IVSCCMiniAnalysisDevelopmentScratchFile to the FTP server

timjarsky avatar Mar 05 '25 19:03 timjarsky

Good news: The deconvoluted data has the same length. Bad news: It has inf at the end.

I'll have a look tomorrow.

grafik

t-b avatar Mar 10 '25 20:03 t-b

@timjarsky This is very likely due to filter settings.

t-b avatar Mar 10 '25 20:03 t-b

@t-b, the latest changes look good excellent! I found just two potential issues. The latter may not be specific to PSX.

  1. I think the fast and slow tau may have been flipped - fast tau returns bigger values than slow tau
  2. The x-axis labels show the last operation/line in a plot with with statements.

image *Note: The color coding on the lower plot was done manually

sel=select(selchannels(AD11),selrange([2000,inf]), selsweeps(22...27),selvis(all))

dat = data($sel)


rtau=2.500000001
dtau=6
amp=-50
SD=1.75
bandfLow=500
bandfHigh=1

kernel=psxkernel($sel, $rtau, $dtau, $amp)

PSXey = psx(CN2275WT_2024_04_23_143529, $kernel, $SD , $bandfLow, $bandfHigh,10,psxRiseTime(),psxDeconvFilter(200, 2, 5))

$PSXey

and

psxstats(CN2275WT_2024_04_23_143529, $sel, weightedtau, acc)
and

psxstats(CN2275WT_2024_04_23_143529, $sel, fasttau, acc)
and

psxstats(CN2275WT_2024_04_23_143529, $sel, slowtau, acc)

and

psxstats(CN2275WT_2024_04_23_143529, $sel, weightedtau, acc, stats)
with

psxstats(CN2275WT_2024_04_23_143529, $sel, fasttau, acc, stats)
with

psxstats(CN2275WT_2024_04_23_143529, $sel, slowtau, acc, stats)

and

psxstats(CN2275WT_2024_04_23_143529, $sel, weightedtau, acc, hist)
with

psxstats(CN2275WT_2024_04_23_143529, $sel, fasttau, acc, hist)
with

psxstats(CN2275WT_2024_04_23_143529, $sel, slowtau, acc, hist)

timjarsky avatar Apr 08 '25 19:04 timjarsky

@t-b I also tested pasting the average fit results into Excel. The row formatting is correct. However, Excel plots all the columns in one column. Excel is dumb. To correct this, paste into Excel, select the cells, select Data from the menu, select Text to Columns, and select the appropriate delimiter in the Excel pop-up GUI.

timjarsky avatar Apr 08 '25 20:04 timjarsky

@t-b there may also be issues with the indivdual decay tau values. For example in this case I'm getting a negative weighted tau.

image

timjarsky avatar Apr 08 '25 20:04 timjarsky

@t-b I also tested pasting the average fit results into Excel. The row formatting is correct. However, Excel plots all the columns in one column. Excel is dumb. To correct this, paste into Excel, select the cells, select Data from the menu, select Text to Columns, and select the appropriate delimiter in the Excel pop-up GUI.

@timjarsky Does PutScrapText "1\t3\r\n2\t4\r\n" result in clipboard data which can be nicely imported into excel?

@t-b there may also be issues with the indivdual decay tau values. For example in this case I'm getting a negative weighted tau.

This is very likely a consequence of the slow/fast tau mixup.

The x-axis labels show the last operation/line in a plot with with statements.

Well that is how it's coded, each psxStats operation sets the x-axis label. Given your SF code we could allow an array of properties in psxStats and create a legend instead. This would also make it easily possible to have distinct colors per property.

t-b avatar Apr 09 '25 20:04 t-b

@t-b

  Message: "results wave got larger unexpectedly"
  Please provide the following information if you contact the MIES developers:
  ################################
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  Stacktrace:
  SF_button_sweepFormula_display(...)#L3016 [MIES_SweepFormula.ipf]
SF_FormulaPlotter(...)#L1922 [MIES_SweepFormula.ipf]
SF_GatherFormulaResults(...)#L1240 [MIES_SweepFormula.ipf]
SF_ExecuteFormula(...)#L6834 [MIES_SweepFormula.ipf]
SF_FormulaExecutor(...)#L1154 [MIES_SweepFormula.ipf]
PSX_OperationStats(...)#L5216 [MIES_SweepFormula_PSX.ipf]
PSX_OperationStatsImpl(...)#L1754 [MIES_SweepFormula_PSX.ipf]
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  Time: 2025-04-10T13:58:27-07:00
  Locked device: [- none -]
  Current sweep: [- none -]
  DAQ: [- none -]
  Testpulse: [- none -]
  Acquisition state: [- none -]
  Experiment: JessicaPSXFileV2 (Packed)
  Igor Pro version: 9.0.6.1 (56565)
  MIES version:
  
  ################################

with

psxstats(CN2275WT_2024_04_23_143529B, $sel, slowtau, un, hist)

File is here

timjarsky avatar Apr 10 '25 21:04 timjarsky

@timjarsky Ready for first try.

t-b avatar Jun 06 '25 17:06 t-b

@timjarsky Ready for first try.

I started playing with it yesterday afternoon :)

timjarsky avatar Jun 06 '25 17:06 timjarsky

@timjarsky Ready for first try.

I started playing with it yesterday afternoon :)

That was without the fancy FFT trick.

t-b avatar Jun 06 '25 17:06 t-b

@t-b I've been thinking about the default values for the sweep filter. It makes sense to use the kernel rise and decay tau to set the defaults with the formula cutoff = 1 / (3 * pi * tau).

I want to do something similar for the deconvolved BP filter, but I'm not sure what we can use to pick those values.

timjarsky avatar Jun 08 '25 22:06 timjarsky

@timjarsky Ready for another round

t-b avatar Jun 10 '25 17:06 t-b

@t-b, the default filter frequencies are tiny. Are the kernel time constants being converted to seconds before calculating the cutoff frequencies?

image

the "block size" control doesn't respect the "current combo" checkbox state

timjarsky avatar Jun 10 '25 18:06 timjarsky

the "block size" control doesn't respect the "current combo" checkbox state

Added to todo list

t-b avatar Jun 10 '25 19:06 t-b

the "block size" control doesn't respect the "current combo" checkbox state

I can't reproduce that, and we also have tests for that in TestBlockIndexLogic which at least pass in main.

t-b avatar Jun 11 '25 15:06 t-b

@timjarsky Given the SF code

trange = [0, inf]
sel = select(selrange($trange),selchannels(AD), selsweeps())
dat = data($sel)

psx(id, psxKernel($sel))

and

store("result name", psxstats(id, $sel, amp, all, count))

which creates the plot

grafik

the following IP code

Function Dostuff()

	string name = "result name"
	WAVE textualResultsValues = GetLogbookWave(LBT_RESULTS, LBN_TEXTUAL_VALUES)

	string key = SFH_FormatResultsKey(SFH_RESULT_TYPE_STORE, name)
	
	WAVE/T/Z settings = GetLastSetting(textualResultsValues, NaN, key, SWEEP_FORMULA_RESULT)
	ASSERT(WaveExists(settings), "Missing settings")
	
	string entry = settings[INDEP_HEADSTAGE]

	//print entry

	WAVE/WAVE/Z parsedData = JSONToWave(entry)
	ASSERT(WaveExists(parsedData), "Missing parsedData")
	ASSERT(IsWaveRefWave(parsedData), "parsedData is not a wave reference wave")

	WAVE/Z element = parsedData[0]
	ASSERT(WaveExists(element), "Missing element")

	print element
End

gathers the the number of events, here 42. For this exact use case you also need the latest version of JSONToWave pushed in this branch.

t-b avatar Jul 09 '25 17:07 t-b

@timjarsky I've fixed the kernelAmp zero error. There was some confusion when calculating the average for multiple combos/ranges.

t-b avatar Jul 29 '25 11:07 t-b

@timjarsky The store example is at https://github.com/AllenInstitute/MIES/pull/2362#issuecomment-3053397726.

t-b avatar Aug 20 '25 10:08 t-b

The conflict resolution is quite hairy. So I've stored the pre-resolution version at: bugfix/2362-psx-support-multiple-selections-before-conflict-resolution

t-b avatar Oct 29 '25 19:10 t-b

@t-b I'm trying to test this PR, but getting confused. I recall that the most recent version of the PSX operation took several other operations as input arguments. In particular, psxsweepBPfilter, which seems to no longer be present in either active PSX branch.

timjarsky avatar Dec 03 '25 23:12 timjarsky

@timjarsky I'll have a look tomorrow.

t-b avatar Dec 03 '25 23:12 t-b

@timjarsky I found something please try again.

t-b avatar Dec 10 '25 14:12 t-b

With the file located here, the following SF code doesn't plot anything or show any errors.

sel=select(selchannels(AD09), selvis(all), selCM(VC), selrange([2000,inf]), selsweeps(10...15))


SD=2.5
TauRise = 0.5
TauDecay = 3
Amp = -10

Kernel = PSXKernel($sel, $TauRise, $TauDecay, $Amp)

Doit = PSX(X2024_04_26_145907AD09, $Kernel, $SD, psxsweepBPfilter(300,2,2), 5, psxRiseTime(),psxDeconvBPFilter())

timjarsky avatar Dec 16 '25 19:12 timjarsky