GH #508 - Remove np.nan values from np.diff array

[ru57y34nn]

Overview:

np.nan values were not being dropped from the end of the np.diff array before calling np.flatnonzero to remove non-zero values in get_stim_epoch. This was causing get_stim_epoch to incorrectly identify the end of the stimulus epoch as the same as the end of the sweep epoch, which was also causing the end of the experiment epoch to be incorrect as well since it relys on the stimulus epoch. By dropping the np.nan values from the end of the difference array (di) in get_stim_epoch, the correct indices for end of the stimulus epoch as well as for the experiment epoch are now being returned. This should fix the issue with sweeps being incorrectly tagged with 'Recording stopped before completing the experiment epoch'.

Addresses:

Addresses issue https://github.com/AllenInstitute/ipfx/issues/508

Type of Fix:

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [ ] Documentation Change

Solution:

After getting the differences from adjacent data points in the current trace (i) with np.diff(), the resulting np.nan values are dropped from the end of the array with ~np.isnan() before the non-zero points are removed with np.flatnonzero(). There are np.nan points at the end of the np.diff() array because there are np.nan values at the end of the current trace (i), and the np.diff() function returns np.nan for the difference between two np.nan points, and then these np.nan points get picked up along with the up/down indices of the stimulus and the index of the last np.nan value will be mistaken as the ending index of the stimulus epoch.

Changes:

Drop np.nan from di array in get_stim_epoch() Updated test_get_stim_epoch() in test_epochs.py to test array containing np.nan Updated Unreleased section of CHANGELOG.md

Validation:

This looks like it was just missed in testing. An array containing np.nan values was not tested in the original unit test. The solution was a pretty simple so the added unit test should be sufficient. *np.nan values can only occur at the end of a sweep so only the ending index of the stimulus epoch was affected.

Screenshots:

See GH issue #508 for screenshots

Unit Tests:

See changes to test_get_stim_epoch() in test_epochs.py

Script to reproduce error and fix:

Configuration details:

Checklist

• [x] My code follows Allen Institute Contribution Guidelines
• [x] My code is unit tested and does not decrease test coverage
• [x] I have performed a self review of my own code
• [ ] My code is well-documented, and the docstrings conform to Numpy Standards
• [ ] I have updated the documentation of the repository where appropriate
• [x] The header on my commit includes the issue number
• [x] My code passes all tests
• [x] I have updated the CHANGELOG.md with the description of changes understandable by end users

Notes:

Use this section to add anything you think worth mentioning to the reader of the issue

[tmchartrand]

@ru57y34nn a better fix for that last issue would be to change the stimulus epoch finding code to correctly recognize the epoch is incomplete, rather than returning out of order indices. https://github.com/AllenInstitute/ipfx/blob/0fc0a7452c831c3bb22e4d7dc3c767d17f9fade5/ipfx/epochs.py#L116 could probably just be changed to look for at least two elements (onset and offset).

[ru57y34nn]

@tmchartrand Thanks for the suggestion, that should be a pretty easy change to make, I'll take a look at it this afternoon. Tim Jarsky pointed out that the epoch information is in the lab notebook of the pxp file and I would like to see if this also makes it into the nwb file. I am thinking that we should try to get the epoch information from the nwb file when possible as the current methods for finding the stimulus epoch will not work very will with more complicated stimulus waves.

[tmchartrand]

Closing as #520 was merged instead.