2.0 feedback - naming patterns

[alexpiet]

My understanding is that metadata 2.0 will change the asset naming pattern from
<platform>_<subject_id>_YYYY-MM-DD_HH-MM-SS
to
<subject_id>_YYYYMMDDTHHMMSSZ (time in UTC)
maybe we will always use pacific time, in which case it would be
<subject_id>_YYYY-MM-DD-THH-MM-SS-08:00 (during PST)
<subject_id>_YYYY-MM-DD-THH-MM-SS-07:00 (during PDT)

EDIT: The proposed update will use local time:
<subject_id>_YYYYMMDDTHHMMSS (local time)

(1) I'm fine with removing the platform prefix. The platform never made sense to me since experiments were often mixed. It's a little annoying to change analysis/data management code.

(2) I strongly object to changing the time encoding pattern!

• The previous format implied the local Seattle time. The new format is explicit about time zones, but leads to either annoying daylight saving time changes. Or we use UTC, in which case referring to assets becomes super confusing. For example, a session recorded at 6pm on April 1st, would have an asset name of April 2nd.
• When I asked @dbirman about this, he mentioned that the change was intended to discourage users from parsing information from the asset name, and instead use the session_start_time field. That argument makes sense, but has two problems. First, people will refer to assets by their name in conversations/manual tracking/debugging. This creates the sort of confusion I mentioned above about time zones. Second, if the intent is to discourage parsing information from the asset name, then why does it matter what format we encode time information in the asset name? So I'm not sure what value this change adds
• Changing the name pattern is going to require a huge amount of code updates. My gut feeling is that this one change will bring an order of magnitude more work than all the other metadata changes combined. For example in the dynamic foraging task, we track mouse progress over multiple days. We will need to update all the acquisition code that loads/saves data, OR use a different naming pattern for data on the local computer than Code Ocean assets. We will need to update our streamlit app that allows users to view mouse behavior. We will need to update my pipeline monitoring code that ensures all scheduled mice run, the data uploaded/processed. We will need to update downstream analysis code as well.
• Changing the day/time pattern will be a much bigger code change than removing the "platform" as we already deal with multiple platform prefixes (behavior, ephys)

[bruno-f-cruz]

I think that there are two issues here regarding datetime formatting.

1. What is the standard?
   Changing to ISO makes sense whether you add timezone information or not. It makes parsing much easier since you are relying on standard libraries (e.g. datetime). While it is annoying for a platform that already exists, the same can be said for something that doesn't exist yet. Suddenly, you need to write and maintain a bespoke parsing string instead of simply typing: datetime.fromisoformat. My preferred solution would be to keep the current standard as a backup for backward compatibility by doing what I suggested in (Consider supporting fromisoformat as the default datetime parser aind-data-schema-models#100). This allows new data acquisition pipelines to move to a standard format but still allows current ones to use the old one as a end-of-life feature.

2. What do to with timezone information
   ISO does not require you to specify timezone. Saskia actually pushed for this idea while we were discussing the standards for filename formats(https://github.com/AllenNeuralDynamics/aind-file-standards/blob/main/core/core-standards.md#filename-conventions). Her suggestion, if I recall correctly, was to consider "local" time if no timezone was provided. Local time is given by some information provided in the metadata. This is not something I would advocate for, but as per the current standards, requiring "Z" in the datetime seems to not be required.

[dbirman]

@alexpiet can you help me assess the upgrade difficulty here. Getting a sense of the upgrade difficulty would help me form a better opinion of whether this should go one way or the other. Specifically I'd like to get a sense of how often this is just datetime.strptime in random places in your code base vs re-used utility functions. Can you point me to some lines in your code where you're doing the name parsing? Thanks!

[dbirman]

(I'm asking because I can't find that many references to strptime in AIND codebases, only a handful of the ones I can find here are related to parsing the name, but of course some repos could be private: https://github.com/search?q=org%3AAllenNeuralDynamics+strptime&type=code)

[alexpiet]

@dbirman, I appreciate you trying to form a quantitative opinion of how much work the transition would be.
@bruno-f-cruz, I appreciate your efforts to build maintainable code and improve software systems. This is valuable work and is important to the institute.

I'm sorry if I'm not communicating effectively. This might be easier to discuss in person.

I think counting instances of strptime (and also strftime) doesn't assess the difficulty. If the only case that needed to be handled was replacing calls from strptime to fromisoformat(), then the transition would be simpler (still annoying and not worth it in my opinion). The biggest problem comes from all the places where we currently have no need to map between formats but due to a change in standard we will now have to create a mapping between standards. 

For example, we have log files, upload manifests, and data saved on the acquisition rigs. In each of those places the name of a session uses the existing format. I have developed many tools that track whether scheduled data acquisition ran correctly, generated errors, generated logs, generated manifests, generated data, uploaded, and processed. Right now, when comparing between all of those things I do not need to ever map formats of session names, or parse session names - everything is in the same format. If we switch to a new standard, then we have a problem. I must do one of the following: update all of my tools to handle/check for both formats, maintain one format in some places and one format in another and update some tools to check/map between them, or reprocess log files/manifests/data files to use the new format. This seems like a complete nightmare. 

The rigs that run this task are in 6 different rooms, managed by a huge mix of people - scientists that run the ephys rigs, RAs that run the training facility. Mice train for 4+ weeks in a core facility, and then are handed off to scientists who run experiments for 1-2 additional weeks. Then mice often have additional procedures like histology. That means some mice will have a mix of session name formats. Coordinating code updates is already a major challenge. If we change formats then managing the project complexity is going to fall on me.  I have no trust that I will receive any support for this transition. This isn't a code complexity problem, it's a project management and people complexity problem. 

FYI, the tools to track data movement live here
https://github.com/AllenNeuralDynamics/behavior_communication/tree/main/log_parser
https://github.com/AllenNeuralDynamics/behavior_communication/tree/main/data_transfer
The acquisition code that loads/saves data lives here
https://github.com/AllenNeuralDynamics/dynamic-foraging-task/blob/main/src/foraging_gui/Foraging.py

Han has built a streamlit app that tracks mouse behavior (https://foraging-behavior-browser.allenneuraldynamics-test.org/). This is used by RAs and scientists. The same problem exists here. Does Han maintain checking for things in multiple formats? Or reprocess log files? We already have to deal with older data in multiple formats. 

I think the problem continues much longer after this asset name transition happens. Scientists are currently analyzing their data. That means they have scripts/notebooks(juypter and physical)/spreadsheets/figures/powerpoint slides that reference specific session names. They might revisit those analyses only to find code no longer functions, data is no longer available. Much of this lives off code-ocean, or uses hard-coded session names (don't blame me). All of that might be easy enough to deal with but the collective time spent of every scientist spending 20 minutes dealing with little one-off issues is huge. Even more, I'm sure that the reaction of many scientists will be to ping me "Hey Alex, I can't find my data, what happened". This isn't a code complexity problem, this is a project management and people complexity problem.

For literally years we have been asking for more engineering support. Without support Han, Xinxin, and myself built infrastructure around the existing AIND tools. Is it perfect? No. Could it have been better architected? Of course. But we feel proud that we built a working system. It feels like we are being double punished here - punished at first for not getting engineering support, punished again by being asked to update our infrastructure because SciComp tools are changing under our feet. 

So then what is the benefit for this huge amount of work? As far as I can tell, the only advantages I have heard is that we can replace this statement in some documentation (where?)
Asset names follow the following format: <subject_id>_YYY-MM-DD_HH-MM-SS
  with this
Asset names follow the following format: <subject_id>_<isoformat time/date, local time>

And future code can use
datetime.fromisoformat(asset_name_with_subject_id_parsed_out)
instead of
datetime.fromisoformat(asset_name_with_subject_id_parsed_out, AIND_FORMAT_STRING)
Note that appending the subject ID to an asset name isn't some international standard, so people must maintain their own parsers anyways! 

Altogether this feels like an absurd change that puts a huge amount of burden on scientists for very little gain. The goal here is to do science, not maintain a perfect code base. We are three years into AIND and we are talking about how we name files?If we were starting from scratch, that would be a different story. The more I think about this, the more strongly I feel that this proposed change does not make any sense.

[dbirman]

I think we're punting this issue downstream by reverting, but we definitely hear your concern that there's a big unknown here about exactly how much code this is going to affect. So we're going to roll back the datetime change while keeping the removal of platform.

One response to your comments

Note that appending the subject ID to an asset name isn't some international standard, so people must maintain their own parsers anyways!

Tell everybody: don't parse names! We really discourage name parsers and tracking names directly in excel spreadsheets, etc! We'll make this more of a focus later in the year, but keep an eye on code that interfaces with the name field or maintains lists of names and try to shift those to use metadata queries going forward so that we're pulling timezone information and recovering accurate datetime objects.

[alexpiet]

Thank you for listening to feedback

[bruno-f-cruz]

But how do we know the standard of the datetime will be ISO without reading some sort of documentation that tells us the standard is ISO? I didn't say the cognitive burden is the same, I said it was similar. In both cases, the user must read documentation to know what format the datetime will be in.
Doesn't the current AIND format also specify the order of MM-DD, and the separators?

I don't think they are similar. For ISO all we have to say is: "All dates are ISO compatible" (maintenance and tools for free). The second one: "AIND follows the following format for dates: YYYY-MM-DD_HH-MM-SS. People are expected to write their own parsers and formaters to ensure interoperability with ISO compliant tools".

I think your logic here doesn't make sense. You are suggesting we should do significant maintenance (adopt a new standard) so that we have the ability to add extra functionality (specify timezone). Your argument for doing this is that if we don't, if we want to add extra functionality, then we will need to do maintenance.

The maintenance cost exists either way! Just because someone already wrote something that implements this bespoke format, it does not mean that everyone else did. Are there not resources that must also be spent such that the current asset format is disseminated to other pipelines that could otherwise simply use standard tools?

The metadata fields already specify time information, we are only discussing the name of the asset. I simply don't see using datetime.isoformat() over datetime.strptime() in future code as a good reason to force entire projects to refactor existing code.

I agree, that's why my preferred solution for common pipelines, if possible, would be:

1. Try to parse from iso
2. If 1) fails, try to parse with the non-standard format
3. if 2) fails raise an error

Unless you require timezone information (which is not planned from what I understood), the information in the datetime is the same.

[dbirman]

This has been reverted.

Don't parse names!