AllenInstitute / AllenSDK

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

Ophys NWB Conversion fails if a dff file is missing #2683

Open Ahad-Allen opened 1 year ago

Ahad-Allen commented 1 year ago

Describe the bug I have already spoken with Adam and Chris about this issue in person, but the crux of the problem is that several planes of sessions from the Dendrite Coupling project seem to be failing cell segmentation despite not being dendrite planes. An example of such a plane can be found here: http://lims2/job_logs?id=1237195071&utf8=%E2%9C%93. This results in a lack of a dff file being generated, making it impossible to currently generate an nwb file from the session.

To Reproduce It is currently not possible to reproduce the exact problem with the current SDK, as we are running a version modified to allow for nonbehavior sessions. That being said, the problem occurs when calling the from_lims function of the ophys experiment class and is specifically caused by the cell_specimens attempting to do a from_lims call. An exact error stack is attached below Traceback (most recent call last): File "/allen/programs/mindscope/workgroups/openscope/ahad/test_cron/OpenScopeNWB-feature-firebase_testing/scripts/ophys_nwb_generation.py", line 159, in <module> gen_ophys(experiment_id, file_path) File "/allen/programs/mindscope/workgroups/openscope/ahad/test_cron/OpenScopeNWB-feature-firebase_testing/scripts/ophys_nwb_generation.py", line 57, in gen_ophys ophys_nwb = ophys.from_lims(ophys_experiment_id=ophys_experiment_id, File "/allen/programs/mindscope/workgroups/openscope/ahad/Conda_env/src/allensdk/allensdk/brain_observatory/behavior/ophys_experiment.py", line 192, in from_lims cell_specimens = CellSpecimens.from_lims( File "/allen/programs/mindscope/workgroups/openscope/ahad/Conda_env/src/allensdk/allensdk/brain_observatory/behavior/data_objects/cell_specimens/cell_specimens.py", line 299, in from_lims dff_traces = _get_dff_traces() File "/allen/programs/mindscope/workgroups/openscope/ahad/Conda_env/src/allensdk/allensdk/brain_observatory/behavior/data_objects/cell_specimens/cell_specimens.py", line 275, in _get_dff_traces dff_file = DFFFile.from_lims( File "/allen/programs/mindscope/workgroups/openscope/ahad/Conda_env/ophys_nwb/lib/python3.8/site-packages/cachetools/__init__.py", line 520, in wrapper v = func(*args, **kwargs) File "/allen/programs/mindscope/workgroups/openscope/ahad/Conda_env/src/allensdk/allensdk/brain_observatory/behavior/data_files/dff_file.py", line 56, in from_lims filepath = db.fetchone(query, strict=True) File "/allen/programs/mindscope/workgroups/openscope/ahad/Conda_env/src/allensdk/allensdk/internal/api/__init__.py", line 52, in fetchone response = one(list(self.select(query).to_dict().values())) File "/allen/programs/mindscope/workgroups/openscope/ahad/Conda_env/src/allensdk/allensdk/__init__.py", line 61, in one raise OneResultExpectedError("Expected length one result, received: " allensdk.OneResultExpectedError: Expected length one result, received: [] results from query

Expected behavior I think an appropriate way of handling this would be creating a blank or placeholder dff file for the affected sessions. Perhaps there are other approaches that might be more relevant.

Additional context Affected IDs: [1251096797, 1239346403, 1239346399, 1239346409, 1239346406, 1239106233, 1239106230, 1239106239, 1239106236, 1238869561, 1238869555, 1238869558, 1238869565, 1237715839, 1237715849, 1237715842, 1237715846, 1237809243, 1237809249, 1237809240, 1237809246, 1237173936, 1237173945, 1237173940, 1237173948, 1234011006, 1234010995, 1234011003, 1234010999, 1232834789, 1232834792, 1232834786, 1232834783]

Do you want to work on this issue? I have no problems helping Adam and Sam test fixes for this or modifying code for them.

morriscb commented 1 year ago

Hey @Ahad-Allen, in terms of packaging and reading from NWB through the SDK, I think the easiest thing for us to implement for you is to just not have the code not write anything for the missing data as opposed to writing None or an empty array say. Would this work for your purposes? This would be for all traces and the cell specimen table.

morriscb commented 1 year ago

Hey @Ahad-Allen. I think I have a working solution for you available on the branch ticket/PSB-117/dev. The changes are only to a single file, cell_specimens.py so should be simple enough to merge into your fork. Could you please test the behavior on your experiment and see if you are able to load from lims and write/load from nwb? I've tested it on my end and it seems to work but we should test it in the context of your fork before calling this done.

Ahad-Allen commented 1 year ago

Perfect, thank you chris! I'll test that out with an experiment that has the dff error missing problem and confirm whether an nwb passes validation.

Ahad-Allen commented 1 year ago

Chris and I met earlier today to try and diagnose this problem and I am attaching the error stack i got when trying to convert a file with the code changes: /allen/programs/mindscope/workgroups/openscope/ahad/Conda_env/src/allensdk/allensdk/brain_observatory/comparison_utils.py:9: FutureWarning: pandas.util.testing is deprecated. Use the functions in the public API at pandas.testing instead. from pandas.util.testing import assert_frame_equal /allen/programs/mindscope/workgroups/openscope/ahad/Conda_env/ophys_nwb/lib/python3.8/site-packages/pandas/io/sql.py:762: UserWarning: pandas only support SQLAlchemy connectable(engine/connection) ordatabase string URI or sqlite3 DBAPI2 connectionother DBAPI2 objects are not tested, please consider using SQLAlchemy warnings.warn( Traceback (most recent call last): File "/allen/programs/mindscope/workgroups/openscope/ahad/test_cron/OpenScopeNWB-feature-firebase_testing/scripts/ophys_nwb_generation.py", line 159, in gen_ophys(experiment_id, file_path) File "/allen/programs/mindscope/workgroups/openscope/ahad/test_cron/OpenScopeNWB-feature-firebase_testing/scripts/ophys_nwb_generation.py", line 59, in gen_ophys ophys_nwb = ophys_nwb.to_nwb() File "/allen/programs/mindscope/workgroups/openscope/ahad/Conda_env/src/allensdk/allensdk/brain_observatory/behavior/ophys_experiment.py", line 101, in to_nwb self._motion_correction.to_nwb(nwbfile=nwbfile) File "/allen/programs/mindscope/workgroups/openscope/ahad/Conda_env/src/allensdk/allensdk/brain_observatory/behavior/data_objects/motion_correction.py", line 51, in to_nwb ophys_timestamps = ophys_module.get_data_interface( File "/allen/programs/mindscope/workgroups/openscope/ahad/Conda_env/ophys_nwb/lib/python3.8/site-packages/hdmf/utils.py", line 580, in func_call return func(args[0], pargs) File "/allen/programs/mindscope/workgroups/openscope/ahad/Conda_env/ophys_nwb/lib/python3.8/site-packages/pynwb/base.py", line 76, in get_data_interface return self.get(data_interface_name) File "/allen/programs/mindscope/workgroups/openscope/ahad/Conda_env/ophys_nwb/lib/python3.8/site-packages/hdmf/utils.py", line 580, in func_call return func(args[0], pargs) File "/allen/programs/mindscope/workgroups/openscope/ahad/Conda_env/ophys_nwb/lib/python3.8/site-packages/hdmf/container.py", line 664, in _func raise KeyError(msg) KeyError: "'dff' not found in data_interfaces of ProcessingModule 'ophys'."

aamster commented 1 year ago

Thanks @Ahad-Allen . Looks like the issue is because we are pulling the ophys timestamps by using the dff traces (which are not written in this case). We'll have to figure out another way to pull the ophys timestamps.

morriscb commented 1 year ago

Hey @Ahad-Allen, I just pushed another commit to the ticket/PSB-117/dev branch. It changes how the motion corrected data is written to NWB. Previously, it tried to load the dff traces data to get the timestamps for the frames, assuming it had been written. I've changed it so that it no longer relies on this input and just reuses the OphysTimestamps object that is already instantiated in the OphysExperiment. Give it a shot and let me know if it works.

Ahad-Allen commented 1 year ago

I was able to generate NWB files successfully using this approach! I am now running them through some validation. Thanks for all the help

morriscb commented 1 year ago

Great! Let us know if you run into anything.

Ahad-Allen commented 1 year ago

Just wanted to confirm that, using this patch, we have converted nwb files, added raw information to them, and uploaded them to dandi. Thank you so much for the assistance!