Insert recreated epoch info on massexport by MichaelHuth · Pull Request #2385 · AllenInstitute/MIES

MichaelHuth · 2025-04-02T16:34:51Z

Needs Enhance SF labnotebook reporting #2432
Needs Conversion/MIES_MassExperimentProcessing.ipf: Fix it #2569
Wait for epoch support in ipfx
Test a full QC run with recreated epoch info on a pxp without epoch info

close #2076

close #881

Close #2009

MichaelHuth · 2025-04-10T16:55:45Z

The commit e26bbaf that includes the TTL Epoch should also solve #881 .

In a future commit we want to reuse the cache invalidation logic for a single logbook. And this also makes InvalidateLBIndexAndRowCaches_Impl superfluous.

ED_AddEntriesToLabnotebook and further down functions were extended to add the feature that a given value is inserted into the LNB at the end of the sweepNumber/entrySourceType block. This row is also marked as postprocessed data. The row and index cache is invalidated when a row was inserted. Added tests for insertion in numerical and textual LNB.

- Added capability SupportsEntrySourceType - Added function to determine that support - Added function to get a LBN capability This should improve performance because capabilities stay constant through the lifetime of the LBN. The information about the EntrySourceType support is required for LBN row insertion. The insertion is done for a specific EntrySourceType at the end of the block defined by sweepNumber/EntrySourceType in the LBN. However, old LBNs do not feature the EntrySourceType column. In the LBN wave upgrade process this column is created but keeps the initialization values of NaN for UNKNOWN_MODE. Now if a row is inserted, we have to insert it with the same sweepNumber/EntrySourceType combination as the block for which FindRange determined the start/end row index. FindRange has a fallback for the old LBN properties. If we would blindly write the given EntrySourceType (which is typically not UNKNOWN_MODE) on insertion in such an old LBN, effectively a different sweepNumber/EntrySourceType combination is created that is then an own block. This new block makes the fallback of FindRange fail to determine the "old" LBN entries boundaries, making them non-retrievable. As the determination of EntrySourceType support is expensive and a static property of the LBN, it can be done in the LBN upgrade process and saved in the wavenote (of the LBN key wave).

- Epochs for DA channels get recreated. - Epoch recreation is an optional argument for NWB_ExportAllData and is by default off. - The location where the LNB is written to the NWB file through NWB_WriteLabnotebooksAndComments was moved to a spot after the LNB was adapted. Added tests for postProcessed epoch addition for vintage files.

…port TTL epochs are now also inserted into the LBN.

We have a test LabnotebookUpgradeDoesNotModifyDefaultWaves which checks that the labnotebook waves are not changed with the upgrade procedure when they have cleared wave notes. This test now fails as HasLBNEntrySourceTypeCapability returns 0 for a values wave with only NaNs. But in this case it does support the entry source type capability. So let's fix it.

This is faster. It does return -1 with no hit, so we have to change the check in the early return as well. We prefer the canonical smaller zero here.

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

Packages/MIES/MIES_ExperimentDocumentation.ipf:119

The row bounds check is off by one: DimSize(values, ROWS) returns the count, so a valid last index is DimSize-1. Using ">= row" would incorrectly allow row == DimSize(values, ROWS) (out of bounds).

static Function ED_SetLabnotebookRowToPostProcessed(WAVE values, variable row)

	variable col, unused

	ASSERT(DimSize(values, ROWS) >= row, "Row does not exist in LBN values wave")

Packages/tests/UTF_HelperFunctions.ipf:2210

This check misses the case where the pre-export epoch version is NaN/missing (which NWB_ExportAllData explicitly treats as requiring recreation). As written, those sweeps are skipped and the helper won’t validate that recreated epoch info was inserted.

		for(name : sweepWaveNames)
			sweep = ExtractSweepNumber(name)
			if(epochVersionsPre[sweep] < SWEEP_EPOCH_VERSION)
				CHECK_EQUAL_VAR(SWEEP_EPOCH_VERSION, epochVersionsAfter[sweep])

+	DFREF  sweepDFR  = GetSingleSweepFolder(deviceDFR, sweepNo)
+	WAVE/Z recEpochs = EP_RecreateEpochsFromLoadedData(numericalValues, textualValues, sweepDFR, sweepNo)
+	if(!WaveExists(recEpochs))
+		print "Could not recreate Epochs."


t-b · 2026-08-06T16:40:28Z

ipfx issue: AllenInstitute/ipfx#518
pynwb epochs docu: https://pynwb.readthedocs.io/en/stable/pynwb.epoch.html#pynwb.epoch.TimeIntervals.add_interval
pynwb example code in MIES:

MIES/tools/nwb-read-tests/nwbv2-read-test.py

Line 69 in 812c062

print(f"epochs: {nwbfile.epochs}")

Copilot

Pull request overview

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

Suppressed comments (3)

Packages/tests/UTF_HelperFunctions.ipf:2208

Missing epoch-version entries are represented as NaN, and NaN < SWEEP_EPOCH_VERSION is false. Consequently this helper skips every assertion for the exact legacy case with no epoch metadata, allowing the export test to pass without verifying that recreation occurred. Match the production condition by handling NaN explicitly.

			if(epochVersionsPre[sweep] < SWEEP_EPOCH_VERSION)

Packages/MIES/MIES_NeuroDataWithoutBorders.ipf:633

The mass-processing path never enables this option: Packages/Conversion/MIES_MassExperimentProcessing.ipf:164 still calls NWB_ExportAllData(...) without recreateEpochs, and this default keeps it false. As a result, the PR's target mass export does not recreate any epochs even though the historic-data test passes the flag explicitly. Forward recreateEpochs = 1 from PerformMiesTasks (or expose and forward a mass-processing option).

	recreateEpochs = ParamIsDefault(recreateEpochs) ? 0 : !!recreateEpochs

Packages/MIES/MIES_Epochs.ipf:1815

The newly recreated TTL epochs are never adapted to the acquired sweep length. The live path adapts enabled TTL channels in EP_AdaptEpochInfo (MIES_Epochs.ipf:1342-1348), but this recreation path only adapts s.DACList below. For an early-terminated sweep, exported TTL epochs can therefore extend past acquired data and omit the unacquired region. Apply the same adaptation to enabled TTL channels before sorting.

	EP_CollectEpochInfoTTL(recEpochWave, s)

+			if(recreateEpochs)
+				epochVersion = GetLastSettingIndep(s.numericalValues, s.sweep, SWEEP_EPOCH_VERSION_ENTRY_KEY, DATA_ACQUISITION_MODE, defValue = NaN)
+				if(IsNaN(epochVersion) || epochVersion < SWEEP_EPOCH_VERSION)
+					InsertRecreatedEpochsIntoLBN(s.numericalValues, s.textualValues, s.device, s.sweep)


+static Function ED_InsertRowAfterSweepBlock(WAVE values, variable sweepNo, variable entrySourceType)
+
+	variable sweepCol, firstRow, lastRow, rowIndex, size
+
+	sweepCol = GetSweepColumn(values)
+	FindRange(values, sweepCol, sweepNo, entrySourceType, firstRow, lastRow)
+	ASSERT(!IsNaN(firstRow) && !IsNaN(lastRow), "FindRange could not determine start and/or end of sweep block")
+	rowIndex = lastRow + 1
+	InsertPoints/M=(ROWS) rowIndex, 1, values
+
+	if(!IsTextWave(values))
+		values[rowIndex][][] = NaN
+	endif
+
+	size = GetNumberFromWaveNote(values, NOTE_INDEX)
+	SetNumberInWaveNote(values, NOTE_INDEX, size + 1)
+
+	InvalidateLBIndexAndRowCache(values)
+
+	return rowIndex
+End


MichaelHuth self-assigned this Apr 2, 2025

MichaelHuth requested review from t-b and timjarsky as code owners April 2, 2025 16:34

MichaelHuth force-pushed the feature/2385-add_epoch_info_on_massexport branch from 466c735 to 0730cc5 Compare April 2, 2025 16:36