Adaptive Supra enhancements #2123

t-b · 2024-05-22T21:29:49Z

PSQ_DS_AdaptiveIsFinished:

46 2925	 if(!WaveExists(sweepPassed) || DimSize(sweepPassed, ROWS) < numSweepsWithSaturation)
0  2926	   return 0
46 2927	endif

PSQ_DAScale

16 3864 if(ret)
0  3865    printf "Could not calculate DAScale step widths min/max.\r"
0  3866    ControlWindowToFront()
0  3867    return 1
16 3868 endif

14  3904 if(!WaveExists(futureDAScales))
11  3905   if(PSQ_DS_AdaptiveIsFinished(device, s.sweepNo, s.headstage, numSweepsWithSaturation, fromRhSuAd = 1))
0  3906     PSQ_ForceSetEvent(device, s.headstage)
0  3907     RA_SkipSweeps(device, Inf, SWEEP_SKIP_AUTO, limitToSetBorder = 1)
0  3908  
0  3909     WAVE/ZZ DAScales
0  3910     break
11  3911   endif
14  3912 endif

Q: Discuss with Tim: We currently determine passing f-I slope for each sweep after the sweep. But it could be the case that the maximum slope changes after deciding that we passed. Do we need to revise our earlier decision?
A: No revision, but our method to determine the sweeps with saturation should change. We only want to consider sweeps with passing f-I slope and passing sweep QC which have a larger DAScale value than the maxSlope sleep.
It should be the case that we always have both sweeps passing for the f-I slope. Check that and assert out otherwise
Q: Check what happens when we have the same DAScale values for a f-I slope fit. A: Don't start the measurement if we only have constant DAScale data in RhSuAd (testcase : PS_DS_AD8). If this happens in the adaptive suprathreshold run this is equivalent to a NaN f-I slope/offset and we just try again. This is also covered in a test.
Check that we display/test the dashboard messages in CI
Fix the fail messages to mention how many f-I slope QC sweeps we were expecting vs being present. ~~Show them in one line as they need to be consecutively (with ignoring failed sweeps).~~
Survey plot: Use DAscale as x-axis for f-I slope data (lower plot)

Requires:

Close #2119
Close #2129
Close #2130
Close #2141
Close #2131

t-b · 2024-07-10T18:48:41Z

@timjarsky Ready for a first test.

We can't trigger a situation where the total fit over all f-I/DaScale values fails but the fit for PSQ_DA_NUM_POINTS_LINE_FIT points passes in PSQ_DS_FitFrequencyCurrentData.

…verride active This catches test setup errors earlier and with a better error message.

Now that we also consider RhSuAdd sweeps for set QC, it does not make sense to enfore that NumSweepsWithSaturation is smaller than the sweeps in the stimset.

We now use the previous acquired (RhSuAd) sweeps for judging if we have enough passing sweeps with f-I slope QC. For this we need to store the list of passing RhSuAd sweeps together with their f-I slope QC status. Calculating the f-I slope QC for these sweeps posed the problem that we have to distinguish sweeps which are left to the maximum slope (usually smaller but not passing) to the ones to the right. We now take that into account in PSQ_DS_CalculateReachedFinalSlope. This made it also required to keep f-I slope values which stem from duplicated apfreq values. These are now added with the value -inf (PSQ_DS_SKIPPED_FI_SLOPE). These changes now can create the situation that we start a adaptive suprathreshold measurement but due to previous RhSuAd sweep already passing enough for our numSweepsWithSaturation we just repeat the last DAScale value and are done. The function PSQ_DS_GetLabnotebookData does all the heavy lifting for getting the desired labnotebook values for the RhSuAd data and the current SCI.

It now does not filter out NaN values and also allows to filter out data values like f-I slope which is calculated for two sweeps.

This does all the heavy lifting for us.

Since we now also look at RhSuAd sweeps we always have at least one sweep passing so the wave is never null.

The condition for a passing f-I slope should actually compare the DAScale values instead of the indizes.

… well This is a defense-in-depth check as fitSlope can never be smaller than NaN, but it expresses the intent better.

…ally available

We can nowadays use a configuration option to achieve it, so let's just prefer that.

…mode

For PSQ_DAScale Adaptive Suprathreshold it can be the case that all sweeps pass but as we don't have enough fI slope QC passes the set still fails. Take that into account by returning an empty string when all sweeps pass.

This has the complication that we need to remove one point from the DAScale array as the slope is always between two values. Also CheckSurveyPlot needs to be revised as the plots now can not be distinguished anymore by the number of traces.

We need to take NumSweepsWithSaturation into account. We also make the fail message easier to grasp with not mentioning the exact slope percentages. And while at it we also reorder the messages to complain about not reaching futureDASCale at the end as this error is quite common.

t-b · 2024-08-03T20:12:18Z

@timjarsky I've rebased against main so that #2213 is included here. Would be good to update the rig computer with the version from the latest CI.

t-b self-assigned this May 22, 2024

t-b changed the title ~~PSQ_DaScale: Add new operation mode Adaptive Suprathreshold~~ Reuse f-I data from previous failing adaptive SCIs May 22, 2024

t-b force-pushed the feature/2123-dascale-adaptive-supra-threshold-reuse-fi-data branch from 21741f8 to 85030ef Compare May 24, 2024 11:13

t-b changed the title ~~Reuse f-I data from previous failing adaptive SCIs~~ Adaptive Supra enhancements Jun 13, 2024

t-b force-pushed the feature/2123-dascale-adaptive-supra-threshold-reuse-fi-data branch from 85030ef to c216078 Compare June 14, 2024 13:46

t-b mentioned this pull request Jun 17, 2024

New Release: 2.8 #2150

Closed

3 tasks

t-b force-pushed the feature/2123-dascale-adaptive-supra-threshold-reuse-fi-data branch from c216078 to 3394779 Compare June 19, 2024 20:31

t-b force-pushed the feature/2123-dascale-adaptive-supra-threshold-reuse-fi-data branch 3 times, most recently from 558c93d to 71c7091 Compare July 10, 2024 18:45

t-b assigned timjarsky and unassigned t-b Jul 10, 2024

t-b marked this pull request as ready for review July 10, 2024 18:48

t-b requested a review from timjarsky as a code owner July 10, 2024 18:48

t-b force-pushed the feature/2123-dascale-adaptive-supra-threshold-reuse-fi-data branch 6 times, most recently from 0873c5e to 07a83bc Compare July 16, 2024 16:18

t-b assigned t-b and unassigned timjarsky Jul 16, 2024

t-b force-pushed the feature/2123-dascale-adaptive-supra-threshold-reuse-fi-data branch 2 times, most recently from aa30210 to 4035595 Compare July 19, 2024 18:01

t-b requested a review from MichaelHuth as a code owner July 19, 2024 18:01

t-b force-pushed the feature/2123-dascale-adaptive-supra-threshold-reuse-fi-data branch 4 times, most recently from 9715991 to 8c403a7 Compare July 26, 2024 19:55

t-b added 23 commits August 3, 2024 22:10

PSQ_DS_CalculateAndStoreDAScaleStepWidths: Remove return value

a3d0448

We can't trigger a situation where the total fit over all f-I/DaScale values fails but the fit for PSQ_DA_NUM_POINTS_LINE_FIT points passes in PSQ_DS_FitFrequencyCurrentData.

PSQ_DS_GetPassingRhSuAdSweepsAndStoreInLBN: Add assertions for test o…

4e09bd0

…verride active This catches test setup errors earlier and with a better error message.

PSQ_DAScale_CheckParam: Don't require a minimum NumSweepsWithSaturation

417445b

Now that we also consider RhSuAdd sweeps for set QC, it does not make sense to enfore that NumSweepsWithSaturation is smaller than the sweeps in the stimset.

PSQ_DS_FilterPassingData: Enhance it

37e7674

It now does not filter out NaN values and also allows to filter out data values like f-I slope which is calculated for two sweeps.

Tests/PSQ_DAScale Adaptive: Verify the survey plot

fc571ba

PSQ_DS_CreateSurveyPlotForUser: Prefer PSQ_DS_GetLabnotebookData

04413c7

This does all the heavy lifting for us.

UTF_PatchSeqDAScale_Adapt.ipf: Add some more tests

235c2ca

PSQ_DS_AdaptiveIsFinished: Remove sweepPassed existence check

844759b

Since we now also look at RhSuAd sweeps we always have at least one sweep passing so the wave is never null.

GenerateAnalysisFunctionTable: Update for changed analysis parameters

4beeb77

PSQ_DS_CalculateReachedFinalSlope: Take DAScale value into account

bc9b4a1

The condition for a passing f-I slope should actually compare the DAScale values instead of the indizes.

PSQ_DS_CalculateReachedFinalSlope: Require that maxSlope is finite as…

36937ed

… well This is a defense-in-depth check as fitSlope can never be smaller than NaN, but it expresses the intent better.

PSQ_DAScale: Make default values of optional analysis parameters glob…

8006e54

…ally available

PSQ_DS_IsValidMode: Make it available for all

92685c4

CheckDashboard: Ensure the message field is filled

e79b367

OpenDatabrowser: Make it static

84e4ac9

We can nowadays use a configuration option to achieve it, so let's just prefer that.

AD_FillWaves: Decorate the analysis function name with the operation …

8a7ddc8

…mode

AD_GetDAScaleFailMsg: Handle PSQ_DaScale without operation mode

857e568

Packages/doc/dot/Readme.md: Add note about printing flowcharts

6c3e8d4

Packages/doc/dot/patch-seq-dascale.dot: Update it

e6bf6b0

t-b force-pushed the feature/2123-dascale-adaptive-supra-threshold-reuse-fi-data branch from e0f0d38 to e6bf6b0 Compare August 3, 2024 20:10

timjarsky approved these changes Aug 5, 2024

View reviewed changes

t-b enabled auto-merge August 5, 2024 19:14

t-b unassigned timjarsky Aug 5, 2024

t-b merged commit 1a0233d into main Aug 6, 2024
24 checks passed

t-b deleted the feature/2123-dascale-adaptive-supra-threshold-reuse-fi-data branch August 6, 2024 01:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adaptive Supra enhancements #2123

Adaptive Supra enhancements #2123

t-b commented May 22, 2024 •

edited

Loading

t-b commented Jul 10, 2024

t-b commented Aug 3, 2024

Adaptive Supra enhancements #2123

Adaptive Supra enhancements #2123

Conversation

t-b commented May 22, 2024 • edited Loading

t-b commented Jul 10, 2024

t-b commented Aug 3, 2024

t-b commented May 22, 2024 •

edited

Loading