Insert recreated epoch info on massexport

[MichaelHuth]

• Needs Enhance SF labnotebook reporting #2432

close #2076

close #881

Close #2009

[MichaelHuth]

@t-b This PR now always recreates the epochs, whereas the issue states epochs should only be recreated if we do not have any epoch data.

However, we recently changed the epoch (re)creation to be sample exact. With the upper approach we would get a mix of old non-sample exact epochs and "new" sample exact recreated epochs possibly on exported older files.

If that mixing is ok, I add the check to only recreate if there is no epoch info already.

[t-b]

However, we recently changed the epoch (re)creation to be sample exact. With the upper approach we would get a mix of old non-sample exact epochs and "new" sample exact recreated epochs possibly on exported older files.

If that mixing is ok, I add the check to only recreate if there is no epoch info already.

I think we want to regenerate if the epoch info is not-present or out-of-date (SWEEP_EPOCH_VERSION).

[t-b]

Not a review just some drive-by-comments:

• ED_InsertPostProcessRow should work more like ED_AddEntriesToLabnotebook. This would give the same speed benefits (writing all entries in one go) but alleviate the caller of the burden of to manage the internals. Or maybe ED_AddEntriesToLabnotebook should gain a flag for adding a post processing entry?
• Your code does not handle unassociated channels, CreateLBNUnassocKey can derive an LBN key for unassociated channels. We support that since 406440a (Merge pull request Add epoch information for unassociated DA channels + display in DB #1783 from AllenInstitute/feature/1783-add_epoch_info_for_unassoc_DA, 2023-06-30).
• The post processed lbn entry should have the unit LABNOTEBOOK_BINARY_UNIT
• You probably want an assertion after FindRange in case this did return NaN for first and last row

[t-b]

I've reapplied the ipt formatting and linting changes. But don't know howto fix the merge conflict.

[MichaelHuth]

@t-b Look slike the double key check for LBN entry adds actually found a case.
In 061808a#diff-3c3244d0695973447e26d7c941860a630c1fd265833753040b49c4b3d7c20520 I introduced "Sampling interval multiplier" at index 26, where an entry of the same key was already existing at 36 (after the change at 38).

Probable fallout:
The writing to the value source wave in DC_DocumentChannelProperty uses FindDimLabel to determine the column, so it writes to 38.

The copying in ED_createWaveNotes runs the indexing upwards, such that the last value written to the LBN is read from source index 38.

Thus, there should be no side effects.

[t-b]

Does that also resolve the #881?

Review:

59c47e7 (DAP: Change comment getter for user comment to retrieve string directly from control, 2025-04-02)

Jeep.

5c982a3 (LBN: Do labnotebook upgrade on most common MIES entry points, 2025-04-02)

• The commit message is not correct:

  • We upgrade on device locking for DAEphys
  • upgrade on every sweep plot (maybe we can even optimize that?)
  • upgrade on NWB export

• I don't see the upgrade on analysis browser data loading in the diff.

970d2ab (bugfix: Handle missing result waves gracefully in Logbookviewer, 2025-04-02)

• textualValues and numericalValues do exist, but they only exist with the default entries which are removed
  from LBV_CleanLogbookParamNames, right? The code change now results in the cache not being used. The problem is that
  the cache is not able to store null wave refs. We could hack our way here with an empty wave and translate the
  empty wave to a null wave ref in LBV_GetAllLogbookParamNames. Or we store a wave ref wave? Or?

6e1855b (ED: Factor out LNB row initialization to ED_InitNewRow, 2025-04-02)

Looks good.

4f01504 (ED: Remove unused variable in ED_FindIndizesAndRedimension, 2025-04-03)

Nice find. Hopefully a future IPT version can help us here as well.

6801110 (ED: Add check against duplicated keys in ED_CheckValuesAndKeys, 2025-04-03)

I'm trying to come up with a faster way, but fail to see one.

• The only possible optimization is to not do anything if keys has only one column
• Oh and missing newline after the ASSERTs before your code.

The reason I think the one column check is relevant is that all analysis function LBN values are added via
ED_AddEntryToLabnotebook which only adds one entry.

dace787 (Bugfix: SweepSettings Key wave had duplicate key "Sampling Interval Multiplier", 2025-04-07)

Nice.

d5e9e9d (CA: Refactor cache key generation for LB row and index cahce to own functions, 2025-04-04)

• Typo in subject line.

Otherwise nice refactoring.

403130a (AFH: Add function to get headstage from channelType/channelNumber, 2025-04-02)

• That should use an elseif instead of the second if or maybe even a switch?

d67d5b0 (ED: Add feature to ED_AddEntriesToLabnotebook for value insertion, 2025-04-02)

• The commit message says "sweep block" but I think this is more like the entry source type block or?

• ED_AddEntriesToLabnotebook I think finally deserves some proper documentation.

• ED_SetLabnotebookRowToPostProcessed: We want newlines after each logical block inside functions.

• ED_SetLabnotebookRowToPostProcessed: Missing assertion for out of range row.

• The headstage contigency mode for PostProcessed is not DEPEND, but ALL, see ED_ParseHeadstageContigencyMode
  how that resolves. If it would be DEPEND you would not set someting in the INDEP_HEADSTAGE layer.

• ED_InsertRowAfterSweepBlock: Two lines for querying and writing NOTE_INDEX please.

• ED_FindIndizesAndRedimension: It does still resize the rows in values, but now you are not setting the updated
  NOTE_INDEX in ED_createWaveNote/ED_createTextNotes anymore when inserting post processing. Maybe you should
  pass insertAsPostProc into ED_FindIndizesAndRedimension and set the note index there if required? Moving
  SetNumberInWaveNote can also be done in a prep commit. There is also a test missing which would have failed
  with this bug, it should be enough if InsertRowForProstProcessingTextual/InsertRowForProstProcessingNumerical
  checks NOTE_INDEX.

• This should also raise LABNOTEBOOK_VERSION.

3e42a4f (NWB: Insert recreated epoch data into LNB on NWB_ExportAllData, 2025-04-02)

• InvalidateLBIndexAndRowCache looks wrong in this commit.

• InsertRecreatedEpochsIntoLBN: The two loops try to be future proof but the inner one is wrong. The
  number of channels depends on the channel type. So I would just drop that and only do it for DAC. Maybe just
  iterate over recEpochs?

• If you use IsAssociatedChannel instead of IsValidHeadstage you can drop the commit as the code explains
  itself.

• Using AFH_GetHeadstageFromChannelNumberAndType is 100% wrong. This can only be used during DAQ in
  analysis functions. Maybe iterate through recEpochs and then fetch the headstage? This seems to be a
  repeating error. Anything we can do to avoid that?

• I would prefer to resize keys and values for unassociated entries as gotassocValues looks odd and then
  have only one call to ED_AddEntriesToLabnotebook.

• The move of NWB_WriteLabnotebooksAndComments is unmotivated, and if done should be separate.

• Dedicated historic test adding post processed epoch

• This misses to set SWEEP_EPOCH_VERSION in the LBN. We need that for future fixing up.

cb88330 (Tests: Always attempt to recreate Epochs in TestExportingDataToNWB, 2025-04-03)

• Motivation?

52abcfc (LBN: Fix typos in description of labnotebook entries, 2025-04-04)

Nice!

[t-b]

@MichaelHuth Did you see my review in #2385 (comment)?

[MichaelHuth]

I dropped
403130a (AFH: Add function to get headstage from channelType/channelNumber, 2025-04-02)

in favor of a LBN based utility function based on the approach used in DataConfigurationRecreation.

[MichaelHuth]

ED_FindIndizesAndRedimension: It does still resize the rows in values, but now you are not setting the updated
NOTE_INDEX in ED_createWaveNote/ED_createTextNotes anymore when inserting post processing. Maybe you should
pass insertAsPostProc into ED_FindIndizesAndRedimension and set the note index there if required? Moving
SetNumberInWaveNote can also be done in a prep commit. There is also a test missing which would have failed
with this bug, it should be enough if InsertRowForProstProcessingTextual/InsertRowForProstProcessingNumerical
checks NOTE_INDEX.

I noticed that, but I did not evaluate this as bug. The only thing that the call to ED_FindIndizesAndRedimension does when called is to increase the row size of the wave by 1, but not the NOTE_INDEX. (The possible column increase is as regular).

The NOTE_INDEX is updated by the caller in any case.

I think ED_FindIndizesAndRedimension should be split in two parts, one that increases the COLS and another that increases the ROWS. For the insertion I only need the COLS increase as I use another function to add a row.

I am open to move the NOTE_INDEX adjustment before the row value update.

[MichaelHuth]

• Optimize upgrade LBN in DB. Move from DB_UpdateSweepPlot to device locking of DB.
• 970d2ab (bugfix: Handle missing result waves gracefully in Logbookviewer, 2025-04-02) Put result in WaveRef wave that also null ref can be pushed to Cache and Retrieve accordingly.

[MichaelHuth]

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

[t-b]

Fixed conflicts.

[Copilot]

Pull Request Overview

This PR implements functionality to insert recreated epoch information during mass export of NWB data. The primary purpose is to ensure epoch information is available in exported NWB files even when the original data lacks complete epoch information.

Key changes:

• Added a new recreateEpochs parameter to NWB_ExportAllData() function
• Enhanced the labnotebook system to support post-processing entries with dedicated "PostProcessed" tracking
• Implemented epoch recreation logic that inserts missing epoch information into the labnotebook before export

Reviewed Changes

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

Show a summary per file

File	Description
UTF_DataGenerators.ipf	Added test data generators for post-processing functionality
UTF_AttemptNWB2ExportOnOldData.ipf	Updated test to enable epoch recreation during export
UTF_Labnotebook.ipf	Added comprehensive tests for post-processing entry insertion
labnotebook_textual_description.itx	Added "PostProcessed" entry description and fixed spelling errors
labnotebook_numerical_description.itx	Added "PostProcessed" entry description and fixed spelling errors
MIES_WaveDataFolderGetters.ipf	Made upgrade function public and refactored cache key generation
MIES_NeuroDataWithoutBorders.ipf	Added epoch recreation logic to export workflow
MIES_MiesUtilities_Logbook.ipf	Added epoch recreation and cache invalidation functions
MIES_LogbookViewer.ipf	Added null check for robustness
MIES_ExperimentDocumentation.ipf	Enhanced labnotebook entry system to support post-processing
MIES_Epochs.ipf	Made epoch recreation function public
MIES_DataBrowser.ipf	Simplified labnotebook upgrade call
MIES_DAEphys.ipf	Simplified labnotebook upgrade and fixed function call
MIES_Constants.ipf	Added constant for post-processing entry key
MIES_Cache.ipf	Added cache key generator functions
MIES_AnalysisFunctionHelpers.ipf	Added helper function for channel-to-headstage mapping

[t-b]

Recovered from borked-up conflict resolution.