AllenInstitute / MIES

Multichannel Igor Electrophysiology Suite
https://alleninstitute.github.io/MIES/user.html
Other
21 stars 6 forks source link

Revise sweepformula select operation #2109

Open MichaelHuth opened 1 month ago

MichaelHuth commented 1 month ago

close #2012

MichaelHuth commented 1 month ago

@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:

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

MichaelHuth commented 1 month ago

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

t-b commented 1 month ago

@timjarsky Ready for playing around.

t-b commented 3 weeks ago

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

I have compiled a list of items to look at:

    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.

Moved to issues:

t-b commented 2 weeks ago

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

t-b commented 2 weeks ago

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

t-b commented 2 weeks ago

@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?

timjarsky commented 2 weeks ago

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

MichaelHuth commented 2 weeks ago

@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:

t-b commented 2 weeks ago

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

That's good enough.

MichaelHuth commented 2 weeks ago

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

selsci:

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

Regarding device and experiment:

useful functions:

t-b commented 1 week ago

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.

MichaelHuth commented 1 week ago

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 commented 1 week ago

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) ?

t-b commented 1 week ago

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 commented 1 week ago

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?

MichaelHuth commented 1 week ago

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
t-b commented 1 week ago

Todo:

selRACIndex:

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

selSetCycleCount:

selSetSweepCount:

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