LorenFrankLab / rec_to_nwb

Data Migration REC -> NWB 2.0 Service
Other
2 stars 8 forks source link

position_originator thinks epoch timestamps out of order #28

Closed jguides closed 2 years ago

jguides commented 2 years ago

I got the following error when generating an nwb file. It looks like the error arises because the position_originator thinks the epoch timestamps are out of order. I don't see anything obviously amiss in the yaml file used to generate this nwb file. The distances between epoch starts below seem reasonable up to the last one. I am aware @shijiegu also posted an error arising in the position_originator, although that error looked slightly different, so I'm posting mine here. Thanks in advance for any help with this.

ipdb> np.diff(first_timestamps)/60

array([ 54.81162518, 21.76272645, 50.42214757, 21.59982565, 53.52381701, 28.49434762, 36.55967264, 10.7667066 , 22.31105215, 57.62421057, 28.51698989, 49.44335831, 22.06270885, 54.13967245, 22.25902193, 53.1217701 , 106.73524576, 145.68806351, 151.27331273, 395.00253439, -772.70757801])

---------------------------------------------------------------------------
AssertionError                            Traceback (most recent call last)
/tmp/ipykernel_3174520/3482281323.py in <module>
     46                               trodes_rec_export_args=trodes_rec_export_args)
     47 
---> 48     content = builder.build_nwb()
     49     print(content)

~/Src/rec_to_nwb/rec_to_nwb/processing/builder/raw_to_nwb_builder.py in build_nwb(self, run_preprocessing, process_mda_valid_time, process_mda_invalid_time, process_pos_valid_time, process_pos_invalid_time)
    224             process_mda_invalid_time=process_mda_invalid_time,
    225             process_pos_valid_time=process_pos_valid_time,
--> 226             process_pos_invalid_time=process_pos_invalid_time)
    227         logger.info('Done...\n')
    228 

~/Src/rec_to_nwb/rec_to_nwb/processing/builder/raw_to_nwb_builder.py in __build_nwb_file(self, process_mda_valid_time, process_mda_invalid_time, process_pos_valid_time, process_pos_invalid_time)
    238             logger.info('Date: {}'.format(date))
    239             nwb_builder = self.get_nwb_builder(date)
--> 240             content = nwb_builder.build()
    241             nwb_builder.write(content)
    242             if self.is_old_dataset:

~/Src/rec_to_nwb/rec_to_nwb/processing/builder/nwb_file_builder.py in build(self)
    369             self.associated_files_originator.make(nwb_content)
    370 
--> 371         self.position_originator.make(nwb_content)
    372 
    373         valid_map_dict = self.__build_corrupted_data_manager()

~/Src/rec_to_nwb/rec_to_nwb/processing/tools/beartype/beartype.py in func_beartyped(__beartype_func, *args, **kwargs)

~/Src/rec_to_nwb/rec_to_nwb/processing/builder/originators/position_originator.py in make(self, nwb_content)
     63         # check if timestamps are in order
     64         first_timestamps = np.asarray(first_timestamps)
---> 65         assert np.all(first_timestamps[:-1] < first_timestamps[1:])
     66 
     67         logger.info('Position: Injecting into Processing Module')

AssertionError: 
edeno commented 2 years ago

Isn't the last timestamp difference negative?

lfrank commented 2 years ago

It is, and presumably that means that the final epoch isn't the final epoch. For my reference, how is the order currently determined?

edeno commented 2 years ago

I haven't dug into it myself (yet), but I believe @shijiegu indicated it was based on the filenames. @jguides , do you have the names of the position files it is trying to order?

jguides commented 2 years ago

Yes:

shijiegu commented 2 years ago

yes, in this case session 18 s10 is 3 characters, and so it will be considered a later session in the old code. My code fix of epoch order is still in the pull request. It enforces the labeled order in file names. If your older data has similar "s10" situations where the name of the epoch is longer than others, it is likely the epochs are out of order in nwb files.

shijiegu commented 2 years ago

Since Eric is busy, I just merged the pull request myself. @edeno, feel free to make further changes after. @jguides you could pull the code.

jguides commented 2 years ago

Thanks Shijie!