MIES icon indicating copy to clipboard operation
MIES copied to clipboard

Revise sweepformula select operation

Open MichaelHuth opened this issue 1 year ago • 65 comments

  • [x] implementation
    • [x] add new tag to data to skip plotting in formula plotter, skip plotting filter.ranges component
    • [x] set data types on sub waves of combined data (multiple data sets of different type)
    • [x] in upmost wave add custom legend text representation of filter.ranges data
    • [x] workaround PSQ_RHEOBASE for sweepQC, ...Dashboard.ipf L342
    • [x] proper SCI handling (setQC only)
    • [x] add to data
    • [x] review play around
    • [x] add to other select dependent operations
    • [x] add second revision sel* operations
  • [x] docu
  • [ ] ~~Modifying ifn SF documentation needs to wait for https://github.com/AllenInstitute/MIES/pull/2023~~
  • [x] After documentation review from TB, the SF notebook can be updated
  • [x] tests

close #2012

MichaelHuth avatar May 13 '24 23:05 MichaelHuth

@t-b I think the SF changes could be technically tested.

Here are some examples:

swp = sweeps([1...10], 23, [30...40], 42)

sel1 = select(channels(AD), sweeps(1,2), selectVis(all), selectIVSCCsweepQC(passed))

sel2 = select(channels(), sweeps(), selectVis(all), selectIVSCCsweepQC(passed))

rng = selectRange(epochs("E1", $sel1))

ep = epochs("E1", $sel1)
lnb = labnotebook(ADC, [$sel1,$sel1])
sel = select($sel1, $sel1)

#tp = tp(tpbase(), [$sel1, $sel1])

sel3 = select($sel1, selectRange($ep + [0, 0]))

#data([$sel1, $sel2])
#data($sel1)
#$lnb

#data([select(), 10]) # fails

#data([select(), abc]) # fails

data([select(), $sel1])

Arrays for the select argument are allowed for tp, labnotebook, data. epochs takes only a single select argument. Since the argument order of select is now free, it was not possible to replace sweeps() with e.g. [1,2]. Thus, sweeps() takes now sweep numbers as arguments like sweeps(1, 2).

Rules:

  • all new select*() operation can appear multiple times in select(...) except selectRange(...). selectRange(...) can appear only once.
  • all select*() operations in select are logical ANDed (e.g. selectVis(all), selectVis(displayed) -> displayed)
  • if select(...) has another select(...) as argument, then all unset filter properties are treated as 'allow all'
  • if select(...) has no other select(...) as argument it uses the default settings as before, e.g. select()
  • specifying selects as arrays for data means data(select()) is the same as data([select()]) and can be furhter extended with data([$sel0, $sel1, $sel2, ...]) where each select is treated independently with its attributed ranges.
  • epochs takes only a single select argument (we can discuss if there is a use case for select arrays here)

The two expressions with the fails comment relate to the adapted formula executor.

MichaelHuth avatar May 28 '24 17:05 MichaelHuth

I am currently looking into an issue where:

ep = epochs(E1, select())
selectRange($ep + [0, 0]) # works
selectRange(epochs(E1, select()) + [0, 0]) # fails parsing

pushed a preliminary fix

MichaelHuth avatar May 29 '24 15:05 MichaelHuth

@timjarsky Ready for playing around.

t-b avatar May 30 '24 12:05 t-b

Tim and I played around today. And it is very nice!

I have compiled a list of items to look at:

  • [x] BSP_CheckProc_ChangedSetting(...)#L1293 [MIES_BrowserSettingsPanel.ipf] UpdateSweepPlot(...)#L6407 [MIES_MiesUtilities.ipf] SB_UpdateSweepPlot(...)#L357 [MIES_AnalysisBrowser_SweepBrowser.ipf] PostPlotTransformations(...)#L3970 [MIES_MiesUtilities.ipf] SF_Update(...)#L2764 [MIES_SweepFormula.ipf] PGC_SetAndActivateControl(...)#L231 [MIES_ProgrammaticGUIControl.ipf] SF_button_sweepFormula_display(...)#L2812 [MIES_SweepFormula.ipf] SF_ExecuteVariableAssignments(...)#L5990 [MIES_SweepFormula.ipf] SF_FormulaExecutor(...)#L1160 [MIES_SweepFormula.ipf] SF_OperationSelect(...)#L4925 [MIES_SweepFormula.ipf] JWN_SetStringInWaveNote(...)#L239 [MIES_JSONWaveNotes.ipf]
    Time: 2024-06-07T15:20:03+02:00
    Experiment: Basic (Packed)
    Igor Pro version: 9.0.6.1 (56657)
    ################################
    
    The line
    
    WN_SetStringInWaveNote(selectResult, SF_META_DATATYPE, SF_DATATYPE_SELECT)
    
    in SF_OperationSelect needs to check if selectResult is null.
    
    
  • [x] selectVis should have a default argument value of "displayed"
  • [x] selectCM should have a default argument value of "all"
  • [x] selectCM(all) does not select anything
  • [x] Change prefix from sub-operations from select to sel to make it shorter
  • [x] Prefix channels and sweeps with sel
  • [x] Typo: sonly
  • [x] SFH_IsValidSingleSelection: Please add a generic function which returns a wave with matches given a wildcard expresssion. There is nothing builtin into IP IIRC only stringmatch and ListMatch.
	for(matchSet : filter.stimsets)
		if(stringmatch(setName, matchSet))
			found = 1
			break
		endif
	endfor
	if(!found)
		return 0
	endif

note: The upper approach can already be done through FindIndizes with property wildcard matching. The utility function will return just true/false as the quoted code does.

  • [x] SFH_IsValidSingleSelection: Can we only fetch the stimset name if required? For the standard case of match pattern * we could avoid that.
  • [x] Allow selectRange() meaning the full range
  • [x] select(channels(), channels(AD), ...) does not work as expected

Moved to issues:

  • Does it make sense to move all operations accepting data operation as input to select arrays?
  • Rename data operation to plot ?! Or better names?

t-b avatar Jun 07 '24 20:06 t-b

@MichaelHuth CI will not run when you have merge conflicts.

t-b avatar Jun 18 '24 17:06 t-b

@MichaelHuth How is the cursor operation handled by the changes here? Would you do selectRange(cursors(A, B))?

t-b avatar Jun 19 '24 17:06 t-b

@timjarsky Does it make sense to add another selector selExp to select the experiment name? This would come in handy if you have sweeps from different experiments in one sweepbrowser. And what about selDev for selecting the device? And selSCI/selRAC?

t-b avatar Jun 19 '24 18:06 t-b

Yes, those all make sense. Thanks @t-b

timjarsky avatar Jun 19 '24 20:06 timjarsky

@MichaelHuth How is the cursor operation handled by the changes here? Would you do selectRange(cursors(A, B))?

cursors is not changed and returns an untyped array.

selRange returns datasets of ranges and has a datatype that allows it to be used with select as the datatype enables to have unordered select arguments. So selrange(cursors(A,B)) gets the cursor range. You can not use cursors as replacement for selrange. And you can only use two cursors in the argument of cursors inside selrange because selrange accepts only arrays with two elements (start / end).

Options would be:

  • give cursors the same data type as selrange and change it to return a dataset. This would allow cursors to be used as replacement for selrange (with exactly two cursors specified).

MichaelHuth avatar Jun 19 '24 22:06 MichaelHuth

So selrange(cursors(A,B)) gets the cursor range.

That's good enough.

t-b avatar Jun 20 '24 08:06 t-b

Add wildcard support for selexp, seldev. Accept only a single match.

selsci:

  • first get N x 3 wave with selections
  • then use sweepNr, channelType, ChannelNumber with selsci to fill up missing SCI sweeps numbers (they have the same channelType/channelNumber)
  • this feature is enabled as soon as selsci() is present, it takes no arguments

selrac: needs only sweep, so extend result from selsweeps right away

Regarding device and experiment:

  • every sweep can originate from a specific device/experiment when loaded in the sweepbrowser
  • useful function: PSX_GenerateComboKey L2955+

useful functions:

  • AFH_GetSweepsFromSameSCI
  • AFH_GetSweepsFromSameRACycle

MichaelHuth avatar Jun 20 '24 16:06 MichaelHuth

I've tried playing around, but I get an assertion when loading data from the analysisbrowser:

PGC_SetAndActivateControl(...)#L231 [MIES_ProgrammaticGUIControl.ipf]
AB_ButtonProc_LoadSweeps(...)#L2749 [MIES_AnalysisBrowser.ipf]
PGC_SetAndActivateControl(...)#L231 [MIES_ProgrammaticGUIControl.ipf]
BSP_ButtonProc_ChangeSweep(...)#L1485 [MIES_BrowserSettingsPanel.ipf]
UpdateSweepPlot(...)#L6410 [MIES_MiesUtilities.ipf]
SB_UpdateSweepPlot(...)#L374 [MIES_AnalysisBrowser_SweepBrowser.ipf]
PostPlotTransformations(...)#L3978 [MIES_MiesUtilities.ipf]
LayoutGraph(...)#L2561 [MIES_MiesUtilities.ipf]
TUD_GetUserDataAsWave(...)#L177 [MIES_TraceUserData.ipf]
GetSetIntersection(...)#L3876 [MIES_Utilities.ipf]
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  Time: 2024-06-26T13:26:17+02:00
  Experiment: Experiment (Packed)
  Igor Pro version: 9.0.6.1 (56657)
  ################################

The bug can also be reproduced with runwithOpts(testsuite = "UTF_TraceUserData.ipf") in Basic.pxp.

t-b avatar Jun 26 '24 11:06 t-b

I've tried playing around, but I get an assertion when loading data from the analysisbrowser:

That was some fallout from the FindIndizes change to double precision as TUD also uses the combination with GetSetIntersection. I have fixed that.

I have added now selsci and selrac.

Currently the performance bottleneck is FindDimLabel is GetLastSetting. It is called very often due to SF_GetSelectData calling GetActiveChannels that iterates over all channels of the type.

MichaelHuth avatar Jun 26 '24 13:06 MichaelHuth

Ready for another try.

I checked also the GetLastSetting DimLabel speed up and for the select case I tried it shows a 20% - 25% performance increase.

selsci now adds the sweeps of the same channeltype/channelnumber, it does not add e.g. the DAC channels of the headstage if through selchannels only ADC was selected. Does that make sense or should it also add the corresponding other channeltype / channelnumber (i.e. always ADC + DAC) ?

MichaelHuth avatar Jun 26 '24 17:06 MichaelHuth

Does that make sense or should it also add the corresponding other channeltype / channelnumber (i.e. always ADC + DAC) ?

Yes that makes sense.

t-b avatar Jun 26 '24 18:06 t-b

I checked also the GetLastSetting DimLabel speed up and for the select case I tried it shows a 20% - 25% performance increase.

Is that percent or percentage points?

t-b avatar Jun 27 '24 11:06 t-b

I checked also the GetLastSetting DimLabel speed up and for the select case I tried it shows a 20% - 25% performance increase.

Is that percent or percentage points?

Results from my "test" in seconds:

FindDimlabel FindValue
RAC 5.7 4.25
SCI 0.3 0.24

MichaelHuth avatar Jun 27 '24 12:06 MichaelHuth

Todo:

  • [x] Rename selSCI/selRAC to selExpandSCI/selExpandRAC
  • [x] Add selSetCycleCount(n), selSetSweepCount(n): LBN "Set Sweep Count" and "Set Cycle Count"
  • [x] Add a way to select the n'th SCI/RAC, do it with selSCIIndex(n) and selRACIndex(n)? Output range of valid sci/rac indizes on error message

selRACIndex:

  • iterate over all loaded experiments
  • get selections as is (sorted by sweep number)
  • determine RAC id for each selection, find n-th different id, return all these selections

selSCIIndex: The index where scis have different id, not the index within an sci.

  • iterate over all loaded experiments
  • get selections as is (sorted by sweep number)
  • get for each selection the headstage
  • in a copy of the headstage wave, zapNaNs then uniques
  • run over all unique headstages -> extract all selections with that headstage -> get SCI id for all these selections
  • return all selections with n-th SCI id and concatenate
  • repeat for next headstage

selSetCycleCount:

  • counts sweeps that are repeated, is LNB entry: "Set Cycle Count"

selSetSweepCount:

  • number of sweep in a stimset (stimset in WB can have multiple sweep waves) "Set Sweep Count"

selSCIIndex: SCI 0: Sweep 0, 1, 2 with headstage at AD0 SCI 1: Sweep 3, 4, 5 same headstage Variant 1: select(selchannels(AD0), selsweeps(0, 3), selSCIIndex(1)) selection: Sweep 0, AD0 (id0) selection: Sweep 3, AD0 (id1) combined with SCI index 1 there is one result, because we have Sweep 3 AD0 with a different SCI id.

Variant 2: we do an implicit SCI expansion select(selchannels(AD0), selsweeps(0, 3), selSCIIndex(1)) selection: Sweep 0, AD0 (id0) Sweep 3, AD0 (id1) combined with SCI index 1 there are three results, because expanding Sweep 3 AD0 adds Sweep 4, 5 and all of these have id1.

Variant 2 can be accomplished by using selExpandSCI(), thus we decide for variant 1.

Order:

  1. selSetCycleCount, selSetSweepCount
  2. selSCIIndex, selRACIndex
  3. selExpandSCI, selExpandRAC

t-b avatar Jun 27 '24 18:06 t-b

I'm still not able to use this due to

  !!! Threadsafe assertion FAILED !!!
  Message: "Wave type mismatch"
  Please provide the following information if you contact the MIES developers:
  ################################
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  Stacktrace:
  AB_ButtonProc_LoadBoth(...)#L2767 [MIES_AnalysisBrowser.ipf]
PGC_SetAndActivateControl(...)#L231 [MIES_ProgrammaticGUIControl.ipf]
AB_ButtonProc_LoadSweeps(...)#L2749 [MIES_AnalysisBrowser.ipf]
PGC_SetAndActivateControl(...)#L231 [MIES_ProgrammaticGUIControl.ipf]
BSP_ButtonProc_ChangeSweep(...)#L1485 [MIES_BrowserSettingsPanel.ipf]
UpdateSweepPlot(...)#L6461 [MIES_MiesUtilities.ipf]
SB_UpdateSweepPlot(...)#L374 [MIES_AnalysisBrowser_SweepBrowser.ipf]
PostPlotTransformations(...)#L4020 [MIES_MiesUtilities.ipf]
LayoutGraph(...)#L2603 [MIES_MiesUtilities.ipf]
TUD_GetUserDataAsWave(...)#L177 [MIES_TraceUserData.ipf]
GetSetIntersection(...)#L3874 [MIES_Utilities.ipf]
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  Time: 2024-07-05T16:09:25+02:00
  Experiment: Untitled ()
  Igor Pro version: 9.0.6.1 (56685)
  ################################

t-b avatar Jul 05 '24 14:07 t-b

I've hacked my way around the assertion.

  • Please regenerate the databrowser macro to avoid the spurious listbox in the main window. The regeneration code is now fixed in main.
  • Can we have a shortcut in SF_IsValidSingleSelection so that Findindizes is not called of filter.stimsets has only one element which is "*".
  • Same for not calling MatchAgainstWildCardPatterns if filter.stimsets has only one element and is "*"
  • Using Gad2-IRES-Cre;Ai14-709273.06.02.02.nwb and loading all sweeps in a sweepbrowser and doing
sel = select(selchannels(AD), selSCIIndex(0), selvis(all), selSweeps())
dat = data($sel)

$dat

I get a bunch of sweeps where I only have expected sweep 0. The result for selRACIndex does also not look right.

selSetSweepCount/setlSetCycleCount are working nicely.

t-b avatar Jul 05 '24 15:07 t-b

@t-b Good for another try.

I fixed the algorithm for the SCI/RAC index (I hope). I could not reproduce the ASSERTion on loading data into the sweepbrowser.

MichaelHuth avatar Jul 09 '24 13:07 MichaelHuth

Some more testing:

Gad2 nwb with all sweeps loaded:

selb = select(selchannels(AD), selvis(all), selSweeps())
selc = select($selb, selstimset("X1PS_SubThres*"), selSCIIndex(0), selRACIndex(1))

data($selc)

gives an RTE in SF_GetSelectDataWithRACorSCIIndex

as cycleIdsZapped in

WAVE cycleIdsZapped = ZapNaNs(cycleIds)

is null.

sela = select(selchannels(AD), selSCIIndex(5), selvis(all), selSweeps())
selb = select(selchannels(AD), selRACIndex(5), selSCIIndex(0), selvis(all), selSweeps())

works as expected.

Databrowser:

  • SF_GetSelectDataWithRACorSCIIndex

    expNames[] = SelectString(isSweepBrowser, DB_EXPNAME_DUMMY, sweepMap[selectData[p][%SWEEPMAPINDEX]][%FileName])

    gives an RTE if used in the databrowser. Reason is that SelectString always evaluates all arguments.

Pvalb nwb with all sweeps loaded:

sela = select(selchannels(AD), selvis(all), selSweeps(), selstimset("X2LP_Search*"))
selb = select($sela, selSCIIndex(0))

data($sela)

and

data($selb)

$selb is empty. If I move the selstimset from sela to selb it works.

selc = select(selchannels(AD), selvis(all), selSweeps(), selstimset("X2LP_Search*"), selSCIIndex(2), selExpandRAC())

seld = select(selchannels(AD), selvis(all), selSweeps(129), selExpandRAC())

sele = select(selchannels(AD), selvis(all), selSweeps(129), selstimset("X2LP_Search*"), selExpandRAC())

IMHO these three selections should all give sweeps 129 - 142.

t-b avatar Jul 10 '24 11:07 t-b

rebased

MichaelHuth avatar Jul 11 '24 09:07 MichaelHuth

Reassigning so that Tim can test it.

t-b avatar Jul 12 '24 21:07 t-b

@MichaelHuth, what is the syntax for wildcard use with selStimset? Thank you.

timjarsky avatar Aug 14 '24 20:08 timjarsky

@MichaelHuth, what is the syntax for wildcard use with selStimset? Thank you.

Internally StringMatch is used and the same wildcard pattern matching applies: DisplayHelpTopic "StringMatch"

If no argument is given to selstimset then "*" is used internally.

MichaelHuth avatar Aug 16 '24 11:08 MichaelHuth

C57BL6J-742780.14.03.03.zip

With the attached NWB file I can't get the following SF to work:

selCommon=select(selchannels(AD0), selvis(all), selCM(IC))
X3prelimSelect=select($selCommon, selStimset("X3*"))
X3epoch=[E0,E2] 
X3epochs=epochs($X3epoch, $X3prelimSelect)# + [[0,10],[0,0]]
selX3=select(selRange($X3epochs), $X3prelimSelect, selivsccsweepqc(passed))

data($selX3)

error: selectData != ranges row number or plus: wave size mismatch if + [[0,10],[0,0]] is commented in

Inserting the short names directly into selX3 works. e.g., selX3=select(selRange([E0,E2]), $X3prelimSelect, selivsccsweepqc(passed))

Nearly identical code applied to a different stimulus set also works:

selCommon=select(selchannels(AD0), selvis(all), selCM(IC))
X1prelimSelect=select($selCommon, selStimset("X1*"))
X1epoch=[E0,E2]
X1epochs=epochs($X1epoch, $X1prelimSelect) + [[0,150],[0,0]]
selX1=select(selRange($X1epochs), $X1prelimSelect, selivsccsweepqc(passed))

timjarsky avatar Aug 16 '24 21:08 timjarsky

@timjarsky @MichaelHuth Michael is looking into the error you are seeing. I've created https://github.com/AllenInstitute/MIES/pull/2238 to enhance the error message.

t-b avatar Aug 19 '24 20:08 t-b

Analysis for the case:

selCommon=select(selchannels(AD0), selvis(all), selCM(IC))
X3prelimSelect=select($selCommon, selStimset("X3*"))
X3epoch=[E0,E2] 
X3epochs=epochs($X3epoch, $X3prelimSelect)# + [[0,10],[0,0]]
selX3=select(selRange($X3epochs), $X3prelimSelect, selivsccsweepqc(passed))

data($selX3)

X3prelimSelect results in sweeps selected 18, 19, 20. Now epochs is applied trying to retrieve ranges for epoch E0 and E2 from all three sweeps. The issue here is that sweep 18 has only epoch E0 and not E2, whereas 19 and 20 have both epochs. Thus, the X3epochs result is a dataset with 3 entries (for sweep 18, 19, 20)

  1. a size (2) array with E0 range from sweep 18
  2. a size (2,2) array with E0, E2 range from sweep 19
  3. a size (2,2) array with E0, E2 range from sweep 20

Now for addition you write a (2,2) array that can be added to dataset 2 and 3 but not for dataset 1.

What could be written instead is something like: X3epochs=epochs($X3epoch, $X3prelimSelect) + dataset([0,10],[[0,10],[0,0]],[[0,10],[0,0]]) which basically created for the addition three datasets with each of the correct data layout that allows an addition.

The error selectData != ranges row number:

In the line selX3=select(selRange($X3epochs), $X3prelimSelect, selivsccsweepqc(passed)) the select results from X3prelimSelect are intersected with the select results from all sweeps where IVSCC sweep QC passed. IVSCC sweep QC did not pass for sweep 18, such that the intersection is 19, 20.

Now with the selRange in the same select statement the three datasets with epoch ranges from sweep 18, 19, 20 are included in the complete select information. When this is evaluated by data($selX3) the error is raised because it has 3 datasets for ranges as input and 2 sweeps.

You might wonder what the difference is when you write selRange([E0,E2]) directly in the select statement instead of resolving it previously through an epochs statement. When resolving through an epochs statement then the resulting data is a numeric array where e.g. E0 is resolved to e.g. [200, 300]. In the upper case the resolved data from epochs created one dataset per selected sweep and the rows in the numeric array in a dataset count the ranges. So if the sweep has E0 and E2 one gets two rows. If this is used e.g. with selRange($X3epochs) in select then it is only included in the complete select information. The ranged extraction of sweep data happens in the data operation.

If instead of the numeric ranges textual ranges (aka epoch names) are included in select then the actual numeric range is determined when the data operation is executed. For the upper case that means if data from sweep 18 is extracted and as range E0, E2 is specified then for E2 there is no range as E2 does not exist. Thus, Sweep 18 / E2 is skipped. So the mapping for selected sweep with given ranges is done one at a time.

When already resolving previously through epochs() to numeric ranges then the mapping must be clear before starting to iterate over the sweep selections. Because if I get 3 datasets with range specifications but have only 2 sweeps selected there is no way to know how to properly map these.

MichaelHuth avatar Aug 20 '24 12:08 MichaelHuth

selCommon=select(selchannels(AD0), selvis(all), selCM(IC))
X1prelimSelect=select($selCommon, selStimset("X1*"))
X1epoch=[E0,E2]
X1epochs=epochs($X1epoch, $X1prelimSelect) + [[0,150],[0,0]]
selX1=select(selRange($X1epochs), $X1prelimSelect, selivsccsweepqc(passed))

works because all of the selections from X1prelimSelect also passed sweep QC, such that the intersection in the selX1 line does not change the number of selected sweeps. (If it would change the number of selected sweeps then it would clash with the number of range from X1epochs, because that refers to X1prelimSelect)

The addition works because all selections have epochs E0 and E2, such that all datasets from epochs contain a (2,2) array.

Some general advice: Try to use for the epochs() statement the same selection as for data(). Otherwise further intersection like in selX1 might change the number of selection and thus, make it impossible to map the ranges to the selections any more.

MichaelHuth avatar Aug 20 '24 12:08 MichaelHuth