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

Enhance adaptive suprathreshold #2302

Open
wants to merge 39 commits into
base: main
Choose a base branch
from
Open

Enhance adaptive suprathreshold #2302

wants to merge 39 commits into from

Conversation

t-b
Copy link
Collaborator

@t-b t-b commented Nov 26, 2024 •
edited
Loading

Close #2258

Next round after 7739d31 (PSQ_DS_FitFrequencyCurrentData: Remove it, 2025-01-28):

  • Merge SF labnotebook: Fix multiple keys with wildcards #2346
  • fix neg pos fillin
  • Add new test for neg pos fillin fix
  • Merge CI: Update ipt #2347
  • Raise PSQ_DA_SCALE_VERSION again
  • MinimumSpikeCount, defaults to 4, minimum (>=) number of spikes required to allow a sweep reaching max slope. We look at current SCI and RhSuAd. -> LBN entry
  • Use different DAScale when we are done due to RhSuAd data -> neg slope method for DAScale estimation
  • Remove invalid slope QC
  • Ignore f-I slope QC == NaN for DAScale estimation
  • Adapt flowchart
  • Raise LABNOTEBOOK_VERSION
  • Look into new file

@t-b t-b self-assigned this Nov 26, 2024
@t-b t-b force-pushed the feature/2302-another-adaptive-revision branch from daec7e1 to 886e9e6 Compare November 27, 2024 22:53
@t-b t-b mentioned this pull request Jan 6, 2025
Open
3 tasks
@t-b t-b force-pushed the feature/2302-another-adaptive-revision branch 2 times, most recently from d7b9af8 to 7a4e927 Compare January 21, 2025 17:56
@t-b t-b force-pushed the feature/2302-another-adaptive-revision branch 3 times, most recently from 87bc99b to 03ebd15 Compare January 26, 2025 20:44
@t-b

This comment was marked as outdated.

Sign in to view
@t-b t-b force-pushed the feature/2302-another-adaptive-revision branch 2 times, most recently from c643b38 to eaa0825 Compare January 27, 2025 18:48
@t-b t-b assigned timjarsky and unassigned t-b Jan 27, 2025
@t-b t-b marked this pull request as ready for review January 27, 2025 18:48
@t-b t-b assigned t-b and unassigned timjarsky Jan 27, 2025
@t-b t-b force-pushed the feature/2302-another-adaptive-revision branch 2 times, most recently from 00b6e6f to ae1e170 Compare January 28, 2025 14:38
@t-b t-b assigned timjarsky and unassigned t-b Jan 28, 2025
@t-b

This comment was marked as outdated.

Sign in to view
@t-b t-b force-pushed the feature/2302-another-adaptive-revision branch from ae1e170 to 17d9733 Compare January 28, 2025 14:45
@t-b t-b self-assigned this Jan 29, 2025
@t-b
Copy link
Collaborator Author

t-b commented Feb 4, 2025 •
edited
Loading

@timjarsky Investigation of the live testing from 2025-01-28.

H25.26.401.11.53.01.01.pxp

No adaptive data

Sst-IRES-Cre.pxp

34-49:

Passing f-I slope QC and sweep QC on sweeps 46, 48 and 49.

58-75:

Passing f-I slope QC and sweep QC on sweeps 72-74. Tricky to see due to +1 offset.

# sweeps = selsweeps(range(34, 49))
sweeps = selsweeps(range(58, 75))
sel = select(selvis(all), $sweeps , selchannels(AD))
mode = UNKNOWN_MODE
entries = ["USER_DA Scale Sweep QC", "USER_Squ. Pul. Sweep QC", "USER_DA Scale f-I neg. slope QC", "USER_DA Scale f-I slope QC", "USER_DA Scale DAScale values left QC", "USER_*Baseline QC", "USER_DA Scale fillin QC"]

labnotebook(["USER_DA Scale f-I slope QC", "USER_DA Scale Sweep QC"], $sel, $mode)

and

labnotebook($entries, $sel, $mode)

and

labnotebook(["Stim Scale Factor"], select($sel, selsweeps(range(64, 75))), $mode)

and

labnotebook(["*f-I slope"], select($sel, selsweeps(range(64, 75))), $mode)

and

anafuncParam("NumSweepsWithSaturation", $sel)

Sst-IRES-Cre (1).pxp

10-26:

Passing f-I slope QC and sweep QC on sweeps 24, 25 and 26.

37-52:

Passing f-I slope QC and sweep QC on sweeps 49-51.

59-68:

Passed because the inital FI slope was invalid because 2x the same DAScale. ???

83-99:

Passed because

97: f-I slope QC
98: neg f-I slope QC

99 is the pos/neg slope fillin, but it has the wrong DAScale value. ???

# sweeps = selsweeps(range(10, 27))
# sweeps = selsweeps(range(37, 53))
sweeps = selsweeps(range(59, 69))
# sweeps = selsweeps(range(83, 100))

sel = select(selvis(all), $sweeps , selchannels(AD))
mode = UNKNOWN_MODE
entries = ["USER_DA Scale Sweep QC", "USER_Squ. Pul. Sweep QC", "USER_DA Scale f-I neg. slope QC", "USER_DA Scale f-I slope QC", "USER_DA Scale DAScale values left QC", "USER_*Baseline QC", "USER_DA Scale fillin QC", "*BL QC"]

labnotebook(["USER_DA Scale f-I slope QC", "USER_DA Scale Sweep QC", "USER_DA Scale f-I slope valid QC"], $sel, $mode)

and

labnotebook($entries, $sel, $mode)

and

labnotebook("*f-I slope", $sel, $mode)

and

labnotebook("Stim Scale Factor", $sel, $mode)

and

labnotebook("USER_DA Scale AP frequency from*", $sel, $mode)

and

labnotebook("USER_DA Scale f-I slopes from*", $sel, $mode)

and

labnotebook("*operation mode", $sel, $mode)

and

labnotebook("*passing sweep numbers*", $sel, $mode)

and

anafuncParam("NumSweepsWithSaturation", $sel)

Sst-IRES-Cre (2).pxp

18-24:

pos/neg slope on 23 and 24. Missing fillin due to bug. ???

35-44:

passing f-I slope on 42-43 (both from RhSuAd) and 44

# sweeps = selsweeps(range(18, 25))
sweeps = selsweeps(range(35, 45))

sel = select(selvis(all), $sweeps , selchannels(AD))
mode = UNKNOWN_MODE
entries = ["USER_DA Scale Sweep QC", "USER_Squ. Pul. Sweep QC", "USER_DA Scale f-I neg. slope QC", "USER_DA Scale f-I slope QC", "USER_DA Scale DAScale values left QC", "USER_*Baseline QC", "USER_DA Scale fillin QC"]

labnotebook(["USER_DA Scale f-I slope QC", "USER_DA Scale Sweep QC"], $sel, $mode)

and

labnotebook($entries, $sel, $mode)

and

labnotebook("*f-I slope", $sel, $mode)

and

labnotebook("Stim Scale Factor", $sel, $mode)

and

labnotebook("*operation mode*", $sel, $mode)

and

labnotebook("*f-I slope QCs*", $sel, $mode)

and

anafuncParam("NumSweepsWithSaturation", $sel)

Sst-IRES-Cre (3).pxp

44-59:

Sweeps 57 and 58 have f-I neg slope QC. 59 is fillin with wrong dascale ???

74-84:

Passed because the inital FI slope was invalid because 2x the same DAScale. ???

# sweeps = selsweeps(range(43, 60))
sweeps = selsweeps(range(74, 85))

sel = select(selvis(all), $sweeps , selchannels(AD))
mode = UNKNOWN_MODE
entries = ["USER_DA Scale Sweep QC", "USER_Squ. Pul. Sweep QC", "USER_DA Scale f-I neg. slope QC", "USER_DA Scale f-I slope QC", "USER_DA Scale DAScale values left QC", "USER_*Baseline QC", "USER_DA Scale fillin QC", "*f-I initial slope valid from*"]

labnotebook(["USER_DA Scale f-I slope QC", "USER_DA Scale Sweep QC"], $sel, $mode)

and

labnotebook($entries, $sel, $mode)

and

labnotebook("*f-I slope", $sel, $mode)

and

labnotebook("Stim Scale Factor", $sel, $mode)

and

labnotebook("*operation mode*", $sel, $mode)

and

labnotebook("*f-I slope QCs*", $sel, $mode)

and

anafuncParam("NumSweepsWithSaturation", $sel)

@t-b t-b force-pushed the feature/2302-another-adaptive-revision branch from 17d9733 to 82df058 Compare February 5, 2025 21:04
@t-b
Copy link
Collaborator Author

t-b commented Feb 11, 2025

H25.03.302.11.53.01.03.pxp:

f-I plot:
grafik

sweeps = selsweeps(range(25, 49))
sel = select(selvis(all), $sweeps , selchannels(AD))
mode = UNKNOWN_MODE
entries = ["USER_DA Scale Sweep QC", "USER_Squ. Pul. Sweep QC", "USER_DA Scale f-I neg. slope QC", "USER_DA Scale f-I slope QC", "USER_DA Scale DAScale values left QC", "USER_*Baseline QC", "USER_DA Scale fillin QC"]

labnotebook(["USER_DA Scale f-I slope QC", "USER_DA Scale Sweep QC"], $sel, $mode)

and

labnotebook($entries, $sel, $mode)

and

labnotebook(["Stim Scale Factor"], $sel)

and

labnotebook(["USER_DA Scale AP frequency"], $sel, $mode)

and

anafuncParam("NumSweepsWithSaturation", $sel)

grafik

t-b added 19 commits February 14, 2025 18:25
@t-b
MIES_AnalysisFunctions_PatchSeq.ipf: Add forgotten PSQ_FMT_LBN_DA_AT_…
fa0333f 
…RSA_FI_OFFSETS
@t-b
PSQ_DS_GetFutureDAScalesFromLBN: Factor it out
9455c9a
@t-b
NumericWaveToList: Fix documentation
99b8554
@t-b
MergeSortStableInplace: Add it
534e8a1 
The stock Igor Pro functions don't maintain the relative order of the same
key values. So we had to roll our own.
@t-b
SortKeyAndData: Add it
2ce1834
@t-b
PSQ_DAScale (Adapt): Return always a copy of the waves for PSQ_DS_Fil…
cd7fc4f 
…terFrequencyCurrentData
@t-b
PSQ_DS_AdaptiveIsFinished: Handle oorDAScale case earlier
6d080da 
In that way this is not dead code in the tests. It is also clearer that we
check this error condition first.
@t-b
PSQ_DAScale (Adapt): Enhance it
028f515 
- Ignore sweeps with negative fI slopes for the DAScale estimation
- Store a new QC value if the fI slope is finite and negative
- Using this new QC we are now also done if:
  - We have to consecutive negative fI slope values (as usual failing
    sweep QC sweeps can be in between)
  - or we have a passing fI slope and a consecutive (as above) negative fI
    slope value
  If we finish due to these two conditions we also fillin another DAScale
  in between the positive FI slope sweep and the first negative fI slope
  value.
- We now use the new analysis parameter DAScaleNegativeSlopePercent,
  defaulting to 50%, to calculate a new DAScale value after a negative
  slope. We use the percentage of the maximum DAScale step for that.
- Add a new labnotebook entry which denotes if a value is filled in or not
- Also a bug is fixed which did not ignore fillin sweeps when trying to
  determine if the set is passing.
- And similiar bug could result in fillin sweeps being marked as fI slope
  passing.
@t-b
UTF_PatchSeqDAScale_Adapt.ipf: Update test matrix
a6678bf
@t-b
PSQ_DA_SCALE_VERSION: Raise it
87f7ca9
@t-b
patch-seq-dascale.dot: Adapt it
44c73e4
@t-b
LABNOTEBOOK_VERSION: Raise it
a802f8b
@t-b
PSQ_DS_FitFrequencyCurrentData: Remove it
533aa5c 
In (PSQ_DAScale: Change fitting logic and drop NumPointsForLineFit,
2024-07-10) we added dropping duplicated AP frequencies in the input data
for fitting. We do catch that now with the neg fI slope QC better. And
that also removes the hack to find a good DAScale in case this was
filtered out.
@t-b
PSQ_DS_NegativefISlopePassingCriteria: Initialize optional variable f…
de97ae0 
…romRhSuAd
@t-b
FindSequenceReverseWrapper: Add it
78ece22
@t-b
PSQ_DS_StoreFitOffsetAndSlope: Allow passed waves to be NULL
115bc28 
This is also expected in the code.
@t-b
GetRowIndex: Handle unknown wave types better
cae856c
@t-b
PSQ_DS_AddDAScaleFillinBetweenPosAndNegSlope: Fix neg pos fillin
53c11ae 
We need to search for a change of the f-I neg slope QC, so going from 0 to
1. Unfortunately FindSequence has a bug when reverse searching, so we need
to workaround that.
@t-b
PSQ_DaScale: Revise it again
9223978 
- Remove the "initial invalid slope QC" criteria for passing a set. This was passing too often for RhSuAd
  data which was obviously not done. We keep the constants though as the dashboard still supports it.
- The analysis function parameter NumInvalidSlopeSweepsAllowed is
  therefore also removed.
- This also required us to handle NaN for f-I slope more gracefully. It is now ignored for the DAScale
  estimaton. In addition we now try to always come up with a new DAScale value, and also check that we don't
  acquire the same one twice.
- When finishing an adaptive set due to the RhSuAd data we now calculate a new DAScale value with the method
  for the negative slope but mark it as PSQ_DS_AD_FINISHED/PSQ_DS_AD_FINISHED_RHSUAD.
- Ignore f-I slope values which are NaN also requires us to make them up again in PSQ_DS_GetLabnotebookData as
  NaN's can not be queried from the labnotebook.
@t-b t-b force-pushed the feature/2302-another-adaptive-revision branch from 82df058 to 9d45c57 Compare February 14, 2025 19:50
t-b added 5 commits February 14, 2025 21:03
@t-b
PSQ_DS_GatherDAScaleFillin: Fix logic to check for already acquired D…
f2e7972 
…AScales

We need to check all of the DAscales waves. In addition the check is OR
not AND which was done for the two neighbours of xm, xp and xm.
@t-b
PSQ_DS_DAScale (Adapt): Add MinimumSpikeCountForMaxSlope analysis par…
c8b3c74 
…ameter

This analysis parameter, defaulting to 4, sets a lower limit for a sweep to
be able to be recognized as having the maximum slope.

This should avoid issues with really small AP frequencies resulting in
large max slopes and therefore an too early stop.
@t-b
GenerateAnalysisFunctionTable: Update it
18af033
@t-b
LABNOTEBOOK_VERSION: Raise it
bc985f5
@t-b
PSQ_DA_SCALE_VERSION: Raise it
1713079
@t-b t-b force-pushed the feature/2302-another-adaptive-revision branch from 9d45c57 to 1713079 Compare February 14, 2025 20:03
@t-b
Copy link
Collaborator Author

t-b commented Feb 14, 2025

@timjarsky Ready for testing. I'll do the flowchart next week. In addition to the requested features I also found an issue with fillin where it would fillin values already acquired in RhSuAd data.

@t-b t-b assigned timjarsky and unassigned t-b Feb 14, 2025
@timjarsky
Copy link
Collaborator

@t-b the latest version is installed on a production rig. No experiments until tomorrow (Feb 18th, 2025).

As part of this PR, could the analysis parameter help messages be standardized? Each message could start with the analysis method it applies to. e.g., sub, supra, adaptive supra, then list the default value and help message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timjarsky timjarsky timjarsky left review comments

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

At least 1 approving review is required to merge this pull request.

Assignees

@timjarsky timjarsky

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Adaptive enhancements
2 participants
@t-b @timjarsky