One fewer timestamp than number of angular changes when calculating angular speed

[Matyasz]

Describe the bug
There are more angular changes than timestamps present in the input to the get_running_df method. For an example, see the LIMS job failures of IDs: 1056414840, 1056294644, 1056188052, or 1056621309. This is all of them at the time of writing this ticket.

Error trace:

Traceback (most recent call last):
File "/allen/aibs/technology/conda/production/allensdk_py36/lib/python3.6/site-packages/allensdk/brain_observatory/behavior/write_nwb/__main__.py", line 122, in write_behavior_ophys_nwb
BehaviorOphysNwbApi(nwb_filepath_inprogress).save(session)
File "/allen/aibs/technology/conda/production/allensdk_py36/lib/python3.6/site-packages/allensdk/brain_observatory/behavior/behavior_ophys_api/behavior_ophys_nwb_api.py", line 52, in save
nwb.add_running_data_df_to_nwbfile(nwbfile, session_object.running_data_df, unit_dict)
File "/allen/aibs/technology/conda/production/allensdk_py36/lib/python3.6/site-packages/allensdk/brain_observatory/behavior/behavior_ophys_session.py", line 237, in running_data_df
self._running_data_df = self.api.get_running_data_df(lowpass=True)
File "/allen/aibs/technology/conda/production/allensdk_py36/lib/python3.6/site-packages/allensdk/api/cache.py", line 75, in wrapper
result = f(*args, **kwargs)
File "/allen/aibs/technology/conda/production/allensdk_py36/lib/python3.6/site-packages/allensdk/internal/api/behavior_ophys_api.py", line 179, in get_running_data_df
return get_running_df(data, stimulus_timestamps, lowpass=lowpass)
File "/allen/aibs/technology/conda/production/allensdk_py36/lib/python3.6/site-packages/allensdk/brain_observatory/behavior/running_processing.py", line 384, in get_running_df
angular_speed = calc_deriv(angular_change, time) # speed in radians/s
File "/allen/aibs/technology/conda/production/allensdk_py36/lib/python3.6/site-packages/allensdk/brain_observatory/behavior/running_processing.py", line 12, in calc_deriv
dxdt_rt = np.hstack((np.nan, dx / dt))
ValueError: operands could not be broadcast together with shapes (270014,) (270013,) 

Environment (please complete the following information):

• AllenSDK version v2.3.1

Tasks:

• Determine if it is acceptable to remove an element from the angular_change, and if not determine another path forward

Additional context
It's worth noting that a warning was raised right before this problem, at this line. In the warning it claims that it will remove the extra value, but it doesn't actually do this. Apparently it has never actually done this. Also, I am concerned as to why the data it is reading in has one fewer timestamp.

[Matyasz]

@dougollerenshaw do you think we should be performing the truncation of the data, like the warning claims it will?

[dougollerenshaw]

Hi @Matyasz , looks like this slipped through the cracks in the running processing refactor! I should have spotted it in review.

A little background. Camstim has long had an issue where it generates one extra encoder reading at the end. See here: AllenInstitute/visual_behavior_analysis#379. This seems to be stochastic on scientifica rigs, but seems to happen every time on mesoscope, which is probably why all of your jobs are now failing.

I submitted a PR to fix this in the previous running processing algorithm: #1474 That included the warning about the off-by-one error, and then instead of actually trimming off the extra data point, it simply indexed into the array using the length of the time array. See here: https://github.com/dougollerenshaw/AllenSDK/blob/620c378f1d849560c84c66e8fd36d602cf4feb1c/allensdk/brain_observatory/behavior/running_processing.py#L45

During the refactor, the warning got carried over, but the indexing fix clearly didn't. It was probably being tested on scientifica sessions without the off-by-one problem, which is how it slipped through.

So, to answer your question, yes. We should trim the data, just like the warning says, if the lengths are off by one.