AllenInstitute / AllenSDK

code for reading and processing Allen Institute for Brain Science data
https://allensdk.readthedocs.io/en/latest/
Other
343 stars 149 forks source link

update 'imaging_depth' definition & calculation for all ophys_experiment objects & cache/summary tables #2545

Closed DowntonCrabby closed 1 year ago

DowntonCrabby commented 2 years ago

Describe the use case that is addressed by this feature. Currently each ophys_experiment has a unique imaging depth that is automatically calculated based on the z depth difference between the average surface images & the ophys imaging depth. This is a column in both the ophys_experiment_table (cache/manifest table) and in the metadata attribute for each ophys_experiment dataset object.

This is fine except that it can become confusing to external users why different ophys experiments in the same ophys container (which contain the same cells) have slightly different depths.

Describe the solution you'd like In the ophys_experiment_table, instead of displaying the actual imaging depth for each ophys experiment, I think it would be more useful to have something like 'target_imaging_depth' or 'ophys_container_depth` some name like that (maybe @matchings can weigh in here) where what we display to users is actually the average depth for all the ophys experiments in an ophys container. This way all the ophys experiments in a container should have the same 'target_depth'.

The metadata attribute for each ophys_experiment could then continue to retain the imaging_depth (perhaps renamed to something else?) AND the new averaged depth calculation as well.

Additional context The new metric will need to be calculated before it can be put in place as this does not exist anywhere else right now.

Do you want to work on this issue? I'm happy to help create a function that will calculate the averaged depth.

mikejhuang commented 1 year ago

@matchings , I updated the OphysExperimentMetadata object to include an attribute named average_container_depth with the requested metric. This metadata contains the entries in which the ophys_experiment_table loads to populate its data.

Here's a sample metadata with the updated field that would be reflected in the 'ophys_experiment_table` Let me know if you think a different name would be clearer.

{
    "equipment_name": "CAM2P.5",
    "sex": "F",
    "age_in_days": 171,
    "stimulus_frame_rate": 60.0,
    "session_type": "OPHYS_1_images_B",
    "date_of_acquisition": "2019-09-25 20:06:45+00:00",
    "reporter_line": "Ai148(TIT2L-GC6f-ICL-tTA2)",
    "cre_line": "Sst-IRES-Cre",
    "behavior_session_uuid": "c6fd4460-1513-408d-8e50-e0c6d45ae1d7",
    "driver_line": [
        "Sst-IRES-Cre"
    ],
    "mouse_id": 470784,
    "full_genotype": "Sst-IRES-Cre/wt;Ai148(TIT2L-GC6f-ICL-tTA2)/wt",
    "behavior_session_id": 955110047,
    "indicator": "GCaMP6f",
    "emission_lambda": 520.0,
    "excitation_lambda": 910.0,
    "ophys_container_id": 941373529,
    "field_of_view_height": 512,
    "field_of_view_width": 447,
    "imaging_depth": 275,
    "average_container_depth": 275,
    "imaging_plane_group": null,
    "imaging_plane_group_count": 0,
    "ophys_experiment_id": 955276580,
    "ophys_frame_rate": 31.0,
    "ophys_session_id": 954981981,
    "project_code": "VisualBehaviorTask1B",
    "targeted_structure": "VISp"
}
matchings commented 1 year ago

@mikejhuang thanks for doing this! and sorry for the slow response.

while average_container_depth does describe the fix we are implementing here, i think it would be better to use a name that is more descriptive of the aspect of the experimental procedure that this value represents. I would prefer to call ittargeted_imaging_depth, as @DowntonCrabby proposed initially. I think this more accurately conveys what the number represents - it is the location in cortex that the ophys operators intended to target across all sessions of a container. It is also more consistent with the name of the related targeted_structure metadata key, which makes it easier for users to understand and associate with each other.

Does that sound ok to you?

matchings commented 1 year ago

@mikejhuang i want to apologize again for my slow response...i just saw that the PR you wrote for this issue was closed about an hour ago using average_container_depth as the name...very sorry i didn't get you the info you needed faster!!

mikejhuang commented 1 year ago

@matchings Sounds good, it is easy to change right now. Thanks for clarifying.