Recreate BLC, BLS user epochs

[MichaelHuth]

requires #2083 requires upcoming LNB readout fix for non-consecutive sweep numbers

[t-b]

If possible I'd like to review this first round this week.

[t-b]

Issue: #2074

Review: fbce383fa (AnaPSQ: Factor out adding epochs for baseline chunk selection, 2024-04-22)

Make sense.

230302798 (EPR: Add BLC epochs for other PSQ functions and BLS epochs, 2024-04-22)

• [x] I don't think anaFuncName and setName are used here.

• [x] Can we define EP_AddRecreatedUserEpochs_GetAnaFuncParams in MIES_AnalysisFunctionHelpers.ipf or another place for general consumption? The generic function should also not throw. Just as a reminder we do have WaveText to spare the wave statement.

• [x] The lines with chunkLengthTime, chunkStartTimeMax are C&P

+            numRequiredEpochs = PSQ_VM_REQUIRED_EPOCHS
+        endif
+        if((type == PSQ_ACC_RES_SMOKE) || (type == PSQ_TRUE_REST_VM))
+            if(!IsNaN(numEpochs) && numEpochs != PSQ_VM_REQUIRED_EPOCHS)
+                return NaN
+            endif

I thought we already discussed about this and you wanted to change it?

• [x] All CreateAnaFuncLBNKey invocations should get a waMode
• [x] Prefer the constant for "U_BLS[0-9]+"

b73e87a47 (AnaPSQ: Refactor SE UserEpoch creation for BLS, TP to dev indep function, 2024-04-23)

Good.

ac0f10dd5 (Ana: Refactor duration calculation to dev independent function, 2024-04-23)

Good.

2d6f7d7a2 (EP: fix EP_GetEpochs causing uncaught RTE when optional epochWave is null, 2024-04-24)

Nice find.

f0304da72 (EP: EP_SortEpochs sort as lowest priority to CRC of tag string, 2024-04-24)

Change is good. But it left the reader puzzled why you used a CRC. Maybe because you can not mix textual and numeric keys for sorting?

2ba5e0870 (EP: Add BLC, BLS epochs and TP epochs for Seal Evaluation as recreated epochs, 2024-04-23)

• [x] The waMode change is not mentioned and should happen in the commit introducing the code.

• [x] DEBUGPRINT("Could not reconstruct durations") should mention why this can happen and refer to an issue as we think we need to fix that as well. I think this is https://github.com/AllenInstitute/MIES/issues/1737.

• [x] This commit also includes other changes to Baseline which should happen separately and/or earlier.

• [x] Don't use FindDuplicates but prefer GetUniqueEntries as the latter also handles one point waves properly.

• [x] TestEpochRecreationRemoveUnsupportedUserEpochs:

  • [ ] TP_.._B1 is missing a trailing $
  • [ ] tpEpochs should only be define in the if block before Concatenate

c95067395 (Tests: Adapt user epoch test for anaFunc, 2024-04-24)

Good!

070091cc3 (Ana: Add note in EP_EvaluateBaselineProperties for epochsWave trick, 2024-04-24)

Nice one.

f4924ab28 (Tests: Adapt HistoricData tests for new supported recreated epochs, 2024-04-30)

Good.

Can you edit https://github.com/AllenInstitute/MIES/issues/2074#issuecomment-2064545684 so that we see which user epochs are still missing?

[MichaelHuth]

@t-b Result for diagnostic for: "find out where RTE is cleared when EP_GetEpochs is called with null wave as epochs wave from SEAL EVALUATION"

The RTE is cleared in IsValidRegexp. The code there is correct. The actual issue is a different one: The calls with a null wave as argument and no /Z flag generate only an RTE if NVAR/SVAR/WAVE checking is enabled. This is only enabled if the debugger is enabled. Thus, when the test case is running a null wave argument does not generate a pending RTE. The code might fail at a later point though, if that is not expected/handled properly. Thus, while being a potential error in the code, we don't see these on a regular test run.

I do not see a solution for that, unless WM changes the IP behavior that NVAR/SVAR/WAVE checking can be enabled without requiring an enabled debugger.

[t-b]

Thanks for the adaptations. LGTM. I'll raise the issue with WM regarding NVAR/SVAR/WAVE checking.