Omitted stimuli bug #1623

pickles-bread-and-butter · 2020-06-15T15:53:17Z

Overview:

Currently Omitted Stimuli exist in Behavior Ophys experiments, these stimuli represent when a experiment subject is being shown a gray screen. These stimuli by design have no ending frame or ending timestamp, they always last for 750ms however. Currently these cannot be written as an NWB interval because the datatype requires an ending timestamp, leaving this information out is not a good idea.

Solution:

The proposed solution that the Visual Behavior team agrees with is to take the starting timestamp, which is present in the omitted stimuli data, and add the standard duration time to get the stop time. This fixes the error where NWB intervals require a stop time, the stop_time is now created and saved in the stimulus table row.

Validation:

The solution has been validated by running through four Behavior Ophys experiments and successfully running the function add_stimulus_presentations, with the inclusion of #1619. The test cases cover the cases where stop_time is not found in a row, any additional testing coverage suggestions would be much appreciated.

Notes:

This does not fully fix the writing of Behavior Ophys NWB files but contributes to eliminating the errors in writing those files!
This PR also adds some documentation to stimulus_processing.py as work was done in this file understanding the omitted stimuli data.

Keys for the name of stimulus are different between eceephys and behavior ophys. This adds in a small function to get the key represented in each data type and raise an exception if one key is not found. This was added to allow ophys behavior experiments to write to nwb files.

I updated the function to be more generic and removed specific case verbaige. I also fixed the failing tests on Linux and Mac. The regex for the tests was returning an error on Linux and Mac.

This function had to be added as the omitted stimuli did not by design have an ending timestamp. These stimuli therfore could not be written to NWB files as the interval data object in PyNWB requires an ending time. This commit adds a function that adds a stop_time to a omitted row that is based off the known time that omitted stimuli are shown. This allows for omitted stimuli types to be written to NWB intervals.

I added documentation for the functions in stimulus_presentations.py that I have touched during investigation. These functions are get_stimulus_presentations and get_visual_stimuli_df. I added doc strings to better explain what is happening in the code.

Corrected import name problem, function name changed and I didn't change it.

codecov-commenter · 2020-06-15T16:31:38Z

Codecov Report

Merging #1623 into address-behavior-ophys-NWB-failures will increase coverage by 0.03%.
The diff coverage is 95.45%.

@@                           Coverage Diff                           @@
##           address-behavior-ophys-NWB-failures    #1623      +/-   ##
=======================================================================
+ Coverage                                35.58%   35.61%   +0.03%     
=======================================================================
  Files                                      344      345       +1     
  Lines                                    33568    33586      +18     
=======================================================================
+ Hits                                     11946    11963      +17     
- Misses                                   21622    21623       +1

Impacted Files	Coverage Δ
allensdk/brain_observatory/nwb/__init__.py	`88.14% <85.71%> (-0.13%)`	⬇️
.../brain_observatory/behavior/stimulus_processing.py	`26.19% <100.00%> (ø)`
allensdk/brain_observatory/nwb/nwb_utils.py	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5200c74...7d1b06c. Read the comment docs.

njmei · 2020-06-17T23:53:17Z

allensdk/brain_observatory/nwb/__init__.py

+
+            # if there is no stop time in the stimuli it is an 'omitted' stimuli
+            # row and therefore by design has no stop_time, we must add this in.
+            if 'stop_time' not in row.keys():


I don't think this is the right place to set omitted stop times. The add_stimulus_presentations function is also used by the Ecephys Neuropixels project and it may not be correct (and probably wrong) to add a stop time if an Ecephys Neuropixels stimulus_presentation table row lacked a stop time.

A better place to set omitted stop times would be when loading the presentation table for ophys+behavior.

I moved the function call to the behavior_ophys_nwb_api module where whole stimulus table is now passed!

Moved logic of checking for omitted stimulus stop_times to behavior_ophys_nwb_api.

Added debug statements to the code.

Fixed small bug used pandas iterrows iterable

Changed debug statements

added further debug statements

added anohter debug statement.

Changed to work on indices of original dataframe and removed debug statements.

small typo

njmei · 2020-06-18T15:42:07Z

allensdk/brain_observatory/nwb/nwb_utils.py

+# All of the omitted stimuli have a duration of 250ms as defined
+# by the Visual Behavior team. For questions about duration contact that
+# team.
+omitted_stimuli_duration = 0.250


This should probably be a default argument to set_omitted_stop_time. e.g.

def set_omitted_stop_time(stimulus_table_row: dict, omitted_stimuli_duration: float = 0.250):

Set it to default and removed module global.

I changed the input for the omitted stimuli stop time setting function to be the whole DataFrame. I updated the tests and confirmed that the function passes the stimuli table writing.

Removed module global variable defining omitted stimuli time in favour of default function arguement.

pickles-bread-and-butter · 2020-06-18T16:31:38Z

@njmei just an FYI the commits look ugly but it's just for you to see the history, I'm going to squash merge when you approve so they won't be carried into the feature branch.

njmei · 2020-06-22T17:27:17Z

allensdk/test/brain_observatory/nwb/test_nwb_utils.py

+def test_set_omitted_stop_time(stimulus_table, expected_stop_time):
+    stimulus_table = pd.DataFrame.from_dict(data=stimulus_table)
+    nwb_utils.set_omitted_stop_time(stimulus_table)
+    assert stimulus_table.iloc[0]['stop_time'] == expected_stop_time


I would prefer this test check that the entire stimulus table is 'correct' rather than checking that the row 0 stop_time is correct.

Updated to test if expected table is equal to created table.

Updated testting to test entire pd dataframe and not specific elements.

njmei

LGTM

* Added function to set the stop_time for omitted stimuli for writing NWB This function had to be added as the omitted stimuli did not by design have an ending timestamp. These stimuli therfore could not be written to NWB files as the interval data object in PyNWB requires an ending time. This commit adds a function that adds a stop_time to a omitted row that is based off the known time that omitted stimuli are shown. This allows for omitted stimuli types to be written to NWB intervals. * Added doc strings to stimulus_presentations.py I added documentation for the functions in stimulus_presentations.py that I have touched during investigation. These functions are get_stimulus_presentations and get_visual_stimuli_df. I added doc strings to better explain what is happening in the code.

pickles-bread-and-butter added 4 commits June 11, 2020 13:18

Updated function to be more generic

39fffca

I updated the function to be more generic and removed specific case verbaige. I also fixed the failing tests on Linux and Mac. The regex for the tests was returning an error on Linux and Mac.

pickles-bread-and-butter force-pushed the omitted-stimuli-bug branch from 5613eb8 to a0604ce Compare June 15, 2020 16:04

Changed import name

1c5bbcd

Corrected import name problem, function name changed and I didn't change it.

pickles-bread-and-butter marked this pull request as ready for review June 15, 2020 16:33

pickles-bread-and-butter changed the base branch from master to address-behavior-ophys-NWB-failures June 16, 2020 18:04

wbwakeman added the braintv relates to Insitute BrainTV program label Jun 16, 2020

njmei requested changes Jun 18, 2020

View reviewed changes

pickles-bread-and-butter added 8 commits June 18, 2020 06:36

Moving logic from NWB writer

c8486ee

Moved logic of checking for omitted stimulus stop_times to behavior_ophys_nwb_api.

debug statements

27842af

Added debug statements to the code.

Update behavior_ophys_nwb_api.py

ca48893

Fixed small bug used pandas iterrows iterable

Update behavior_ophys_nwb_api.py

65cef33

Changed debug statements

debug statements

321f6a1

added further debug statements

Update behavior_ophys_nwb_api.py

7188679

added anohter debug statement.

Changed logic to work on indexes

867badf

Changed to work on indices of original dataframe and removed debug statements.

Update behavior_ophys_nwb_api.py

6364e18

small typo

njmei reviewed Jun 18, 2020

View reviewed changes

pickles-bread-and-butter added 2 commits June 18, 2020 09:24

Changed to Pandas DataFrame Input

594a998

I changed the input for the omitted stimuli stop time setting function to be the whole DataFrame. I updated the tests and confirmed that the function passes the stimuli table writing.

Chagned omitted time duration to default arg

e81df07

Removed module global variable defining omitted stimuli time in favour of default function arguement.

pickles-bread-and-butter requested a review from njmei June 18, 2020 16:30

njmei reviewed Jun 22, 2020

View reviewed changes

Update test_nwb_utils.py

7d1b06c

Updated testting to test entire pd dataframe and not specific elements.

pickles-bread-and-butter requested a review from njmei June 23, 2020 14:19

njmei approved these changes Jun 23, 2020

View reviewed changes

pickles-bread-and-butter merged commit f489520 into address-behavior-ophys-NWB-failures Jun 23, 2020

pickles-bread-and-butter deleted the omitted-stimuli-bug branch June 24, 2020 18:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Omitted stimuli bug #1623

Omitted stimuli bug #1623

Uh oh!

pickles-bread-and-butter commented Jun 15, 2020 •

edited

Loading

Uh oh!

codecov-commenter commented Jun 15, 2020 •

edited

Loading

Uh oh!

njmei Jun 17, 2020

Uh oh!

pickles-bread-and-butter Jun 18, 2020

Uh oh!

njmei Jun 18, 2020

Uh oh!

pickles-bread-and-butter Jun 18, 2020

Uh oh!

pickles-bread-and-butter commented Jun 18, 2020

Uh oh!

njmei Jun 22, 2020 •

edited

Loading

Uh oh!

pickles-bread-and-butter Jun 23, 2020

Uh oh!

njmei left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Omitted stimuli bug #1623

Omitted stimuli bug #1623

Uh oh!

Conversation

pickles-bread-and-butter commented Jun 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview:

Solution:

Validation:

Notes:

Uh oh!

codecov-commenter commented Jun 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

njmei Jun 17, 2020

Choose a reason for hiding this comment

Uh oh!

pickles-bread-and-butter Jun 18, 2020

Choose a reason for hiding this comment

Uh oh!

njmei Jun 18, 2020

Choose a reason for hiding this comment

Uh oh!

pickles-bread-and-butter Jun 18, 2020

Choose a reason for hiding this comment

Uh oh!

pickles-bread-and-butter commented Jun 18, 2020

Uh oh!

njmei Jun 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pickles-bread-and-butter Jun 23, 2020

Choose a reason for hiding this comment

Uh oh!

njmei left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pickles-bread-and-butter commented Jun 15, 2020 •

edited

Loading

codecov-commenter commented Jun 15, 2020 •

edited

Loading

njmei Jun 22, 2020 •

edited

Loading