-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
extended_stimulus_presentations reward_rate calculation is incorrect #759
Comments
@matchings @dougollerenshaw @yavorska-iryna This bug scares me because it calls into question other fields in extended stimulus processing! Yikes! |
@alexpiet can you specify how you loaded the |
I agree with @matchings. Some more specifics would be helpful. |
Here is a minimal working example of the discrepancy for the "rewarded" column. Spot checking, it might only happen on some sessions.
As far as the units for reward_rate, I determined that by visual inspection of the code, it needs to be divided by 0.75s like the |
OK, I'm able to replicate the mismatch in the reward count (164 vs 138). Investigating now. I had to add imports and fix one parentheses typo to get your code block to run. Here's a full working version for reference: from visual_behavior.data_access import loading
import numpy as np
oeid = 1010556662
session = loading.get_ophys_dataset(oeid, include_invalid_rois=False)
esp = session.extended_stimulus_presentations
print(np.sum(esp['rewarded'])) # 164
print(len(session.rewards)) #138 |
yes, sorry I dropped the imports. My head is in a different codebase right now, and I was just flagging this issue for later |
How come your code block has nice colors, and mine doesnt? |
I just learned a new trick from @jsiegle recently. You can make github render your code with Python specific syntax highlighting by doing this: Which gives: import some_package
print(some_variable) |
rewarded
doesn't match rewards table. I get this by comparinglen(session.rewards)
andnp.sum(esp['rewarded'])
reward_rate
are in rewards/image, not rewards/secondThe text was updated successfully, but these errors were encountered: