PSQ_DA_Scale Adaptive enhancements by t-b · Pull Request #2680 · AllenInstitute/MIES

t-b · 2026-03-25T20:17:48Z

Function TextMarkerDemo()

	Variable numPoints = 10

	Make/O/N=(numPoints) xw = p
	Make/O/N=(numPoints) yw = sin(p / 3)

	// text wave supplying the label shown at each data point
	Make/T/O/N=(numPoints) labelw
	labelw = "P" + num2str(p)

	Display yw vs xw
	ModifyGraph mode=3
	// textMarker(trace) = {sourceWave, font, style, rotation, justification, xOffset, yOffset}
	ModifyGraph textMarker(yw)={labelw, "default", 0, 0, 5, 0, 12}
End

Close #2654
Close #2656

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

t-b · 2026-03-30T18:45:20Z

Sweep 82 has pos f-I slope but should have negative slope as we should ignore sweeps with neg f-I slope for slope calculation.

Since e8917b8 (PSQ_DAScale (Adapt): Enhance it, 2025-01-22) (from #2302) we have not only USER_DA Scale f-I slope but also USER_DA Scale f-I slope for DAScale estimation the latter ignoring sweeps with neg f-I slope QC passing.

SF code:

trange = E1
sel = select(selrange($trange),selchannels(AD), selsweeps())
dat = data($sel)
apfreq = apfrequency($dat)
stimscale = labnotebook("Stim Scale Factor", $sel)
fiSlope = labnotebook("USER_DA Scale f-I slope", $sel, UNKNOWN_MODE)
fiSlopeDAScale = labnotebook("USER_DA Scale f-I slope for DAScale estimation", $sel, UNKNOWN_MODE)
$apfreq

and

merge($apfreq)

vs

merge($stimscale)

and

$fiSlope

and

$fiSlopeDAScale

and

merge($fiSlope)

vs

merge($stimscale)

and

$stimscale

and

labnotebook(["User_DA Scale Sweep QC", "USER_DA Scale f-I slope QC", "USER_DA Scale f-I neg. slope QC"], $sel, UNKNOWN_MODE)


and

labnotebook("USER_DA Scale DAScale values left", $sel, UNKNOWN_MODE)

and

labnotebook("USER_DA Scale operation mode", $sel, UNKNOWN_MODE)

and

labnotebook("Set Sweep count", $sel)

and

anaFuncParam("NumSweepsWithSaturation", $sel)

So let's chat about what needs fixing here.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-09T17:30:37Z

+			// ???
 			filterPassedRhSuAd[] = !negSlopesPassedRhSuAd[p] && !IsNaN(fISlopesRhSuAd)


filterPassedRhSuAd[] uses !IsNaN(fISlopesRhSuAd) without indexing ([p]). That likely evaluates incorrectly (or can fail to compile) and will not filter NaNs element-wise as intended. Use !IsNaN(fISlopesRhSuAd[p]) (and consider removing the // ??? placeholder comment once the intended logic is confirmed).

Suggested change

// ???

filterPassedRhSuAd[] = !negSlopesPassedRhSuAd[p] && !IsNaN(fISlopesRhSuAd)

filterPassedRhSuAd[] = !negSlopesPassedRhSuAd[p] && !IsNaN(fISlopesRhSuAd[p])

Copilot · 2026-04-09T17:30:38Z

 	for(i = 0; i < numCheckEntries; i += 1)
-		// checkQC[i] holds the value between sweep i and i + 1
-		if(!refQC[i + 1])
-			continue
-		endif
-
-		// from here on all sweeps pass
 		if(!checkQC[i])
-			numFound = 0
+			numCheckFound = 0
+			numRefFound   = 0
 			continue
 		endif

-		numFound += 1
+		numCheckFound += 1
+		numRefFound   += (refQC[i + 1] == 1)
 	endfor

-	return numFound
+	return numCheckFound >= numCheckRequired && numRefFound >= numRefRequired


PSQ_DS_ConsecutivePasses accumulates numRefFound across the entire current run of checkQC==1, but never resets when refQC[i+1] is 0. This can return true even if there is no contiguous window of numCheckRequired passing checkQC entries that also satisfies the numRefRequired condition (e.g. alternating refQC pass/fail). Consider evaluating the refQC requirement within the last numCheckRequired checks (sliding window) or resetting the counters when the ref requirement is violated for the intended mode.

Copilot · 2026-04-09T17:30:38Z

 /// @brief Return the number of consecutive passing labnotebook entries `checkQC` which all have `refQC` passing
 ///
 /// It is assumed that `checkQC` is a value which is calculated for two neighbouring sweeps.
-static Function PSQ_DS_ConsecutivePasses(WAVE refQC, WAVE checkQC)
+static Function PSQ_DS_ConsecutivePasses(WAVE refQC, WAVE checkQC, variable numRefRequired, variable numCheckRequired)



The docstring for PSQ_DS_ConsecutivePasses still says it returns “the number of consecutive passing … entries”, but the function now returns a boolean expression. Please update the docstring (and any callers/comments relying on a count) to match the new semantics.

Copilot · 2026-04-09T17:30:38Z

+	[WAVE apFreqRef, WAVE apFreqFromRhSuAd, WAVE DAScalesFromRhSuAd, WAVE sweepPassedFRomRhSuAd] = ExtractRefValuesFromOverride(sweepNo, baselineQC = entries[%baselinePass])
+
+	CHECK_EQUAL_WAVES(entries[%apfreq], apFreqRef, mode = WAVE_DATA)
+	CHECK_EQUAL_WAVES(entries[%apFreqFromRhSuAd], apFreqFromRhSuAd, mode = WAVE_DATA)
+	CHECK_EQUAL_WAVES(entries[%dascaleFromRhSuAd], DAScalesFromRhSuAd, mode = WAVE_DATA)
+	CHECK_EQUAL_WAVES(entries[%sweepPassFromRhSuAd], sweepPassedFRomRhSuAd, mode = WAVE_DATA)


Minor naming typo: sweepPassedFRomRhSuAd has inconsistent capitalization (FRom). Renaming to sweepPassedFromRhSuAd (and updating the following use) would improve readability and avoid future copy/paste mistakes.

Suggested change

[WAVE apFreqRef, WAVE apFreqFromRhSuAd, WAVE DAScalesFromRhSuAd, WAVE sweepPassedFRomRhSuAd] = ExtractRefValuesFromOverride(sweepNo, baselineQC = entries[%baselinePass])

CHECK_EQUAL_WAVES(entries[%apfreq], apFreqRef, mode = WAVE_DATA)

CHECK_EQUAL_WAVES(entries[%apFreqFromRhSuAd], apFreqFromRhSuAd, mode = WAVE_DATA)

CHECK_EQUAL_WAVES(entries[%dascaleFromRhSuAd], DAScalesFromRhSuAd, mode = WAVE_DATA)

CHECK_EQUAL_WAVES(entries[%sweepPassFromRhSuAd], sweepPassedFRomRhSuAd, mode = WAVE_DATA)

[WAVE apFreqRef, WAVE apFreqFromRhSuAd, WAVE DAScalesFromRhSuAd, WAVE sweepPassedFromRhSuAd] = ExtractRefValuesFromOverride(sweepNo, baselineQC = entries[%baselinePass])

CHECK_EQUAL_WAVES(entries[%apfreq], apFreqRef, mode = WAVE_DATA)

CHECK_EQUAL_WAVES(entries[%apFreqFromRhSuAd], apFreqFromRhSuAd, mode = WAVE_DATA)

CHECK_EQUAL_WAVES(entries[%dascaleFromRhSuAd], DAScalesFromRhSuAd, mode = WAVE_DATA)

CHECK_EQUAL_WAVES(entries[%sweepPassFromRhSuAd], sweepPassedFromRhSuAd, mode = WAVE_DATA)

t-b · 2026-04-20T19:23:15Z

Look into files posted by Megan in the autopatch-pipeline channel

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+	FAIL()
+


 /// @brief Return the number of consecutive passing labnotebook entries `checkQC` which all have `refQC` passing
 ///
 /// It is assumed that `checkQC` is a value which is calculated for two neighbouring sweeps.
-static Function PSQ_DS_ConsecutivePasses(WAVE refQC, WAVE checkQC)
+static Function PSQ_DS_ConsecutivePasses(WAVE refQC, WAVE checkQC, variable numRefRequired, variable numCheckRequired)

-	variable i, numFound, numCheckEntries
+	variable i, numCheckFound, numRefFound, numCheckEntries

 	// refQC is determined between sweeps so we have always one sweep more than refQC
 	ASSERT(DimSize(refQC, ROWS) == (DimSize(checkQC, ROWS) + 1), "Unmatched refQC and checkQC waves")
+	ASSERT(IsInteger(numRefRequired) && numRefRequired > 0, "numRefRequired must be a strictly positive integer")
+	ASSERT(IsInteger(numCheckRequired) && numCheckRequired > 0, "numCheckRequired must be a strictly positive integer")

 	numCheckEntries = DimSize(checkQC, ROWS)
 	for(i = 0; i < numCheckEntries; i += 1)
-		// checkQC[i] holds the value between sweep i and i + 1
-		if(!refQC[i + 1])
-			continue
-		endif
-
-		// from here on all sweeps pass
 		if(!checkQC[i])
-			numFound = 0
+			numCheckFound = 0
+			numRefFound   = 0
 			continue
 		endif

-		numFound += 1
+		numCheckFound += 1
+		numRefFound   += (refQC[i + 1] == 1)
 	endfor


 	idx = FindSequenceReverseWrapper(seq, negSlopePassedSorted)

 	if(idx >= 0)
-		// -1 because negSlopePassedSorted is calculated for two sweeps
-		centerDAScale = round((DAScaleSorted[idx - ((idx == 0) ? 0 : 1)] + DAScaleSorted[idx]) / 2)
+		centerDAScale = round((DAScaleSorted[idx] + DAScaleSorted[idx + 1]) / 2)
 	endif


+	if(!sweepPassedAll[last] && negSlopePassedAll[last - 1])
+		// split the difference of the last sweep with passed sweep QC and positive
+		// fitSlope and the current one
+		for(i = numEntries - 1; i >= 0; i -= 1)



 			// we want to filter out the sweeps with passing negative slope QC, so invert it
-			filterPassedRhSuAd[] = !negSlopesPassedRhSuAd[p] && !IsNaN(fISlopesRhSuAd)
+			// ???


+				// and if that does not exist we definitly don't have NaN fiSlopes
+				if(!WaveExists(fISlopesRhSuAdLBN))
+					WAVE/T fISlopesRhSuAdLBN = LBN_GetTextWave()


+				// and if that does not exist we definitly don't have a negSlopePassed situation
+				if(!WaveExists(negSlopesPassedRhSuAdLBN))
+					WAVE/T negSlopesPassedRhSuAdLBN = LBN_GetTextWave()
+					Make/FREE/N=(DimSize(dataRhSuAd, ROWS) - 1) negSlopesPassedRhSuAdContents = 0
+					negSlopesPassedRhSuAdLBN[INDEP_HEADSTAGE] = NumericWaveToList(negSlopesPassedRhSuAdContents, ";")


timjarsky · 2026-05-01T20:45:19Z

@t-b, it looks like the "fill-in" post-double-negative slope is occurring between the two negative-slope points (the pink dot is between the blue and orange dots). The fill-in point (pink dot) should be between the yellow and blue dots.

The file H26.51.006.21.62.02.zip is on your FTP

Also, it looks like the DA scale value following the first negative slope is too large - the DAScaleNegativeSlopePercent was set to 2.

t-b · 2026-05-11T16:42:11Z

@timjarsky I've fixed a bug in the newly introduced code which explains what we are seeing.

timjarsky · 2026-05-11T18:25:09Z

@t-b, the latest version is on Rig 6 for testing.

timjarsky · 2026-05-11T22:24:28Z

@t-b IVSCCAPfrequencyTesting.pxp with the latest experiment acquired on a rig is on the FTP.

Can you remind me if fill-in happens continuously or at the end?

The large current injection at sweep 67 seems odd, as it occurred after a negative slope.

Can you have a look?

t-b · 2026-05-12T13:52:29Z

@t-b IVSCCAPfrequencyTesting.pxp with the latest experiment acquired on a rig is on the FTP.

Thanks for the quick turnaround. I'll look into it ASAP.

We want to have deterministic behaviour.

We need to correct when we go from fI slope indizes to DAScale indizes. Bug introduced in bc9b4a1 (PSQ_DS_CalculateReachedFinalSlope: Take DAScale value into account, 2024-07-26).

This makes it possible to use the Debugger on the sub expressions.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

+	PASS()
+
+	return NaN
+


 				if(!WaveExists(fISlope))
-					Make/FREE/N=(DimSize(negSlopePassed, ROWS)) fISlope = NaN
+					Make/D/FREE/N=(DimSize(negSlopePassed, ROWS)) fISlope = 0
+				elseif(beforeSweepQCResult)
+					// see comment above
+					fiSlope[Inf] = 0


 	numEntries = DimSize(DAScaleAll, ROWS)
+	last       = numEntries - 1
+
+	// special treatment for the case that the last sweep has failing sweep QC and passing neg f-I slope
+	//
+	// we don't need to check fillin QC as that is already done in
+	// PSQ_DS_CalculateNegativeSlopePass
+	if(!sweepPassedAll[last] && negSlopePassedAll[last - 1])
+		// split the difference of the last sweep with passed sweep QC and positive
+		// fitSlope and the current one
+		for(i = numEntries - 1; i >= 0; i -= 1)
+			if(sweepPassedAll[i] && fitSlopeAll[i - 1] > 0)
+				return [NaN, NaN, NaN, DAScaleAll[last], apfreqAll[last], DAScaleAll[i]]
+			endif
+		endfor
+
+		// just use the previous sweep
+		return [NaN, NaN, NaN, DAScaleAll[last], apfreqAll[last], DAScaleAll[last - 1]]
+	endif


t-b · 2026-07-07T21:49:23Z

The replay error from #2680 (comment) revealed an adaptive bug :) And both the adaptive bug and replay error are now fixed.

The search is by default case insensitive, so passing options = 2 just selects whole word and removes ABC (the first entry). The latter CHECK_EQUAL_STR with abc does pass as this is case insensitive by default. Fix that. RTWE1D_CombinedOptionsWithAll was equally broken. Introduced in ed26a88 (Add comprehensive unit tests for missing functions in UTF_Utils_WaveHandling.ipf, 2026-02-03).

But we still default to search without tolerance.

…flag

The difference between 1e-14 and 1e-12 is not relevant here.

Otherwise we miss finding values larger than 1000. Bug introduced in e8917b8 (PSQ_DAScale (Adapt): Enhance it, 2025-01-22) and 9940687 (PSQ_DS_GatherDAScaleFillin: Fix logic to check for already acquired DAScales, 2025-02-14).

t-b self-assigned this Mar 25, 2026

Copilot AI review requested due to automatic review settings March 25, 2026 20:17

Copilot started reviewing on behalf of t-b March 25, 2026 20:18 View session

This comment was marked as outdated.

Sign in to view

Copilot AI review requested due to automatic review settings March 25, 2026 20:33

t-b force-pushed the feature/2680-adaptive-enhancements branch from 4126e9e to 1e3ecf1 Compare March 25, 2026 20:33

Copilot started reviewing on behalf of t-b March 25, 2026 20:33 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

Comment thread Packages/MIES/MIES_AnalysisFunctions_PatchSeq.ipf

Comment thread Packages/tests/HardwareAnalysisFunctions/UTF_PatchSeqDAScale_Adapt.ipf Outdated

t-b force-pushed the feature/2680-adaptive-enhancements branch from 1e3ecf1 to b72b436 Compare April 8, 2026 21:52

Copilot AI review requested due to automatic review settings April 9, 2026 17:25

Copilot started reviewing on behalf of t-b April 9, 2026 17:26 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings April 27, 2026 16:03

Copilot AI reviewed Apr 27, 2026

View reviewed changes

t-b assigned timjarsky and unassigned t-b Apr 30, 2026

timjarsky assigned t-b and unassigned timjarsky May 1, 2026

t-b force-pushed the feature/2680-adaptive-enhancements branch from a141a60 to 59491a6 Compare May 11, 2026 16:40

t-b assigned timjarsky and unassigned t-b May 11, 2026

t-b assigned t-b and unassigned timjarsky May 12, 2026

t-b added 14 commits July 7, 2026 13:10

changes

06f3c8e

changes

6c882e6

changes

1d81750

changes

4d801c3

changes

872ec0d

changes

4147552

changes

4582976

changes

a62c5ae

PSQ_DS_GatherDAScaleFillin: Use a stable sorting algorithm

c3c15fe

We want to have deterministic behaviour.

PSQ_DS_IsValidFitSlopePosition: Fix indizes

73a01a2

We need to correct when we go from fI slope indizes to DAScale indizes. Bug introduced in bc9b4a1 (PSQ_DS_CalculateReachedFinalSlope: Take DAScale value into account, 2024-07-26).

PSQ_DS_CalculateReachedFinalSlope: Factor out boolean expresssion

96e1658

This makes it possible to use the Debugger on the sub expressions.

MergeSortStableInplace: Mention that this sorting algorithm is stable

a175f27

Raise Version

c9d575b

changes

0e63571

Copilot AI review requested due to automatic review settings July 7, 2026 11:12

t-b force-pushed the feature/2680-adaptive-enhancements branch from 75be5d2 to 0e63571 Compare July 7, 2026 11:12

Copilot started reviewing on behalf of t-b July 7, 2026 11:13 View session

Copilot AI reviewed Jul 7, 2026

View reviewed changes

t-b added 9 commits July 7, 2026 23:50

treewide: Introduce constants for FindValue/TXOP

33941b0

GetRowIndex: Add documentation

c83e0c4

GetRowIndex: Allow to search with tolerance

d8c4ec5

But we still default to search without tolerance.

tools/check-code.sh: Check for numerical literals for FindValue/TXOP …

474e123

…flag

DEFAULT_TOL: Define it and prefer it

acc9682

MIES_AnalysisFunctions_PatchSeq.ipf: Switch to DEFAULT_TOL

cec44ab

The difference between 1e-14 and 1e-12 is not relevant here.

PSQ_DA_SCALE_VERSION: Raise it

2fe71cf

t-b assigned t-b and unassigned timjarsky Jul 7, 2026

		// ???
		filterPassedRhSuAd[] = !negSlopesPassedRhSuAd[p] && !IsNaN(fISlopesRhSuAd)

Uh oh!

Conversation

t-b commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

t-b commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

t-b commented Apr 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

timjarsky commented May 1, 2026

Uh oh!

t-b commented May 11, 2026

Uh oh!

timjarsky commented May 11, 2026

Uh oh!

timjarsky commented May 11, 2026

Uh oh!

t-b commented May 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

t-b commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

t-b commented Mar 25, 2026 •

edited

Loading

t-b commented Mar 30, 2026 •

edited

Loading