Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal] Automatically align starting_time or timestamps to session_start_time for interfaces that automatically extract the latter #597

Open
h-mayorquin opened this issue Oct 16, 2023 · 5 comments

Comments

@h-mayorquin
Copy link
Collaborator

h-mayorquin commented Oct 16, 2023

For some interfaces like SpikeGLX we automatically extract the session_start_time. If the latter is a dateime object we can automatically shift/align them if the user specifes the session_start_time in the metadata.

The code would be something like this in their add_object method.

    # Shift to the session start time
    metadata = metadata or self.get_metadata()
    interface_session_start_time = extract_session_start_time(self.file_path)   # This is the method of the interface
    conversion_session_start_time = metadata["NWBFile"]["session_start_time"]  # This is the converter metadata which might differ from the interface
    starting_time = interface_session_start_time.timestamp() - conversion_session_start_time.timestamp()
    # Shift timestamps if they are available
    timestamps_shifted = timestamps + starting_time

That way, if some interface / experiment is shifted (i.e. was started let's say 10 minutes after the beginning of the experiment) neuroconv will automatically align them.

What are some downsides on this?

@CodyCBakerPhD
Copy link
Member

There are some concerns I'd potentially have about feasibility but they all require speculation...

Can you provide a full real example of what this would look like now, and what an 'automated' sync method would look like as an alternative?

@h-mayorquin
Copy link
Collaborator Author

The current state of affairs:
Let's say a user as an interface with SpikeGLX and some ad-hoc behavior where the "correct" session_start_time is the one from behavior. For example, we started recording a video before we started the electrophysiology system. Let's say 10 minutes before. The user uses something like the code example from the conversion gallery:

https://neuroconv.readthedocs.io/en/main/conversion_examples_gallery/combinations/ecephys_pose_estimation.html

And correctly sets the session_start_time as indicated:

metadata["NWBFile"].update(session_start_time=behavior_start_time)

Then, if they run the code as in the gallery without doing anything else, then timestamps (or the starting_time) of the ElectricalSeries will be assumed to start from 0 and the resulting nwbfile wrong.

If they wanted to correct the time alignment, they would have to get discover that we have the time setter methods and then modify the timestamps of the SpikeGLX interface by shifting it ten minutes with the method set_aligned_starting_time.

Now, with the method that I am proposing this will be fixed automatically in some cases by calling this code before adding the object to the file.

    # Shift to the session start time
    metadata = metadata or self.get_metadata()
    interface_session_start_time = extract_session_start_time(self.file_path)   # This is the method of the interface
    conversion_session_start_time = metadata["NWBFile"]["session_start_time"]  # This is the converter metadata which might differs from the interface
    starting_time = interface_session_start_time.timestamp() - conversion_session_start_time.timestamp()
    # Shift timestamps if they are available
    timestamps_shifted = timestamps + starting_time
   ... 
   # Set the corrected timestamps (or starting_time)

We can do that with your function set_aligned_timestamps in some places or manually as I do in:
#598

# Shift to the session start time
metadata = metadata or self.get_metadata()
interface_session_start_time = extract_session_start_time(self.file_path)
conversion_session_start_time = metadata["NWBFile"]["session_start_time"]
starting_time = interface_session_start_time.timestamp() - conversion_session_start_time.timestamp()
timestamps_shifted = timestamps + starting_time

@CodyCBakerPhD
Copy link
Member

My question with your setup is; how does the user specify which interface to consider as the 'ground truth' start time? What would the helper function API look like? There is a reason we have left so much of this up to user control to do the lines you are suggesting

It will also only be possible in cases where the resolution of interacting interfaces is the same (some session start times are only 'dates' rather than 'datetimes') which again is why we let custom code specify how to handle different resolutions

@h-mayorquin
Copy link
Collaborator Author

h-mayorquin commented Oct 17, 2023

The ground truth will be the session_start_time of the nwbfile.

I have not think on the exact API yet. We could have this logic inside get_timestamps or get_start_time getter methods so those values are already return adjusted. Not very clear.

Yes, it will only be available for interfaces that have datetimes. For those it would be an improvement, for the rest, there is nothing we could do.

@h-mayorquin
Copy link
Collaborator Author

@CodyCBakerPhD comment in #598 (comment)

In general we've also never encouraged relying on system times as cached by headers for calculating exact start times

Problem: what if SpikeGLX header (from a separate device) has a clock that is different from that of the FicTrac system (on yet another difference device); comparing the system timestamps is not trustworthy at all in this case, and even if the user wanted to do it they would be encouraged to do so via custom code outside of the interfaces (at the custom converter level or conversion script) to verify that they are OK with that approach - I don't think we should do it automatically in this case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants