Skip to content
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

change time distribution needs fixed #115

Closed
dougollerenshaw opened this issue May 8, 2018 · 12 comments
Closed

change time distribution needs fixed #115

dougollerenshaw opened this issue May 8, 2018 · 12 comments
Assignees

Comments

@dougollerenshaw
Copy link
Contributor

Two validation functions are currently failing due to issues with the change distribution. We need to find a way to deal with these. The most likely solution is to switch from drawing change times on a continuous exponential distribution to drawing on a discrete distribution based on the expected number of stimulus flashes in the stimulus window.

Failing validation functions:
Function: validate_max_change_time
Reason for failure: If the change falls on the last flash, it can fall slightly outside of the stimulus window (in the example below, the max change time is 8.256 seconds and the stimulus window is 8.250 seconds)

Function: validate_monotonically_decreasing_number_of_change_times
Reason for failure: The mapping from the continuous distribution to the discrete flashes distorts the exponential function. See the example below.

Histogram of change times from \allen\aibs\mpe\Software\data\behavior\validation\stage_4\doc_images_9364d72_PerfectDoCMouse.pkl

change_time_distribution

@derricw
Copy link

derricw commented May 9, 2018

The problem we have to solve with this is we can't guarantee 100% the number of stimulus flashes that are going to be in the window.

@dougollerenshaw
Copy link
Contributor Author

Yeah, that seems thorny. Should we make the stimulus window just a bit longer so that, even with a dropped frame or two, the number of flashes in the window will still be consistent? Or is the only way to deal with this to define the number of flashes explicitly?

This deserves some time in front of a whiteboard with a handful of us. Probably should not be prioritized this week unless we manage to get everything else solved.

@derricw
Copy link

derricw commented May 9, 2018

We could specify the stimulus window in # of flashes instead of seconds, maybe?

@dougollerenshaw
Copy link
Contributor Author

Yeah, that sounds like the right solution. That would dovetail with explicitly defining the change time by flash number.

Is a major refactor necessary to do that?

@derricw
Copy link

derricw commented Jul 26, 2018

Hey guys,
what behavior do you want when the stimulus doesn't flash? Do we want to pick the time in the same way that we currently do?

@dougollerenshaw
Copy link
Contributor Author

dougollerenshaw commented Jul 26, 2018

That makes sense to me. Since we're already passing a parameter that defines the change time distribution (change_time_dist), perhaps behavior should remain as-is when it's specified as 'exponential'. The geometric distribution could be applied only when 'geometric' is passed.

We'll also need a way to specify the probability value (p) that defines the distribution. I'm agnostic on whether we use the existing parameter that specifies the mean of the exponential distribution (change_time_scale), or if there is a new parameter name. Re-using the parameter has the advantage of limiting the number of parameters, but the disadvantage of making the naming inconsistent with the application. @neuromusic: thoughts?

@derricw
Copy link

derricw commented Jul 26, 2018

we could just call it change_scale and have it apply to both?

@dougollerenshaw
Copy link
Contributor Author

That sounds like a good solution to me, but I have a feeling that @neuromusic will object to changing the existing parameter name due to backward compatibility issues in the data reading functions. Let's give him a chance to weigh in before making a call on that. Can you keep moving forward in the meantime?

@dougollerenshaw
Copy link
Contributor Author

dougollerenshaw commented Jul 26, 2018

Also, in case it's helpful, I just trimmed down the code I used to generate the example plot I showed on Confluence to describe the geometric distribution. This is the bare minimum:

import numpy as np
p=0.2
first_flash = 3 # first flash at which a change can occur (zero indexed)
last_flash = 11 # last flash at which a change can occur (zero indexed)
change_times_by_flash_number = np.random.geometric(p=p,size=10000)+first_flash-1
change_times_by_flash_number = change_times_by_flash_number[change_times_by_flash_number <= last_flash]

Which gives you a histogram of change times that looks like this:

image

@dougollerenshaw
Copy link
Contributor Author

Whoops, a couple of initial mistakes in the plot above. Had to edit, but it should be correct now. This gives you a vector of flash numbers in a trial on which to show a change. This could be created at session start, then drawn from randomly throughout the session.

@derricw
Copy link

derricw commented Jul 31, 2018

This is implemented in 0.3.21.

@dougollerenshaw
Copy link
Contributor Author

This looks good. A couple notes on parameters after a discussion with Justin:

  • the old code effectively pushed the minimum change time right by one flash-blank cycle. We should make the geometric distribution start on at 3.0 seconds to match
  • The old code gave a mean change time of ~4.25 seconds. A geometric p of 0.3 gets us close to that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants