-
Notifications
You must be signed in to change notification settings - Fork 34
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
VEP display #296
VEP display #296
Conversation
…ing layout elements.
…d as the responsibility of a VEPStim object. Added some debugging to report on stim timing
…cept demo for constructing a complex polygon with holes that may be used to generate a checkerboard using fewer visual stim.
…rboard, making rendering more efficient
circle.draw() | ||
|
||
|
||
def draw_positions(win: visual.Window, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like we decide the boundaries of the boxes here. Is there any way to extract the coordinates of individual symbols in a box?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This demo code is useful for visualizing a list of positions to se where they are at in the window. For example, I used this to confirm the checkerboard positions and layouts. It could also be used for matrix symbol positions. But to extract coordinates within a box I think you would need to add some interactivity.
I think it's probably better, though, to compute positions whenever possible. The layout module is useful for this. For instance, I could create a Layout based on the box corners and used its functions to compute the center, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of great work here!
stim_font='Consolas', | ||
timing=(1, 0.5, 4), # prompt, fixation, stimuli | ||
stim_font='Arial', | ||
timing=(1, 0.5, 2, 4), # prompt, fixation, animation, stimuli |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we test these in a timing test? Only asking because I am worried there might be a lag as in Matrix case, where we set the stimulus duration as 0.25 but it ends up ~0.30s
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably want to create a timing task specific to VEP. I had to modify how I represented the checkerboards because it was taking more than the 1 second to display the sequence. However, I based this decision on the experiment clock timings. It would be better to confirm these values with the photodiode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more about how the do_inquiry
method is implemented in the displays. We will time-test this code 👍🏼
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an excellent start for VEP. Thanks for the PR! Should the next task be a time-test calibration?
units: str = 'norm') -> Tuple[float, float]: | ||
"""Scales the provided height value to reflect the aspect ratio of a | ||
visual.Window. Used for creating squared stimulus. Returns (w,h) tuple""" | ||
if units == 'height': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should standardize to one unit type and assert that in our display... I use height in the inquiry preview, but it might be worth a refactor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In practice I have been using norms for everything. When I finish working on the ticket for matrix row heights it should be easy to refactor Inquiry Preview to use that function to compute positions in 'norm' units.
|
||
info = InformationProperties( | ||
info_color=['White'], | ||
info_pos=[(-.5, -.75)], | ||
info_height=[0.1], | ||
info_font=['Consolas'], | ||
info_font=['Arial'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to use monospaced fonts—another possible display type assertion.
win = visual.Window(size=[700, 700], fullscr=False, screen=1, allowGUI=False, | ||
allowStencil=False, monitor='testMonitor', color='black', | ||
colorSpace='rgb', blendMode='avg', | ||
win = visual.Window(size=[700, 700], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know you didn't add this (feel free to kick to a ticket), but we should use:
window_parameters = {
'full_screen': False,
'window_height': 500,
'window_width': 500,
'stim_screen': 0,
'background_color': 'black'
}
win = init_display_window(window_parameters)
waitBlanking=False, | ||
winType='pyglet') | ||
win.recordFrameIntervals = True | ||
frameRate = win.getActualFrameRate() | ||
|
||
print(f'Monitor refresh rate: {frameRate} Hz') | ||
|
||
clock = core.Clock() | ||
box_colors = ['#00FF80', '#FFFFB3', '#CB99FF', '#FB8072', '#80B1D3', '#FF8232'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are these colors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These were in the pivotal tracker ticket. I think when we implement the calibration task we should add a parameter for these.
stim_font='Consolas', | ||
timing=(1, 0.5, 4), # prompt, fixation, stimuli | ||
stim_font='Arial', | ||
timing=(1, 0.5, 2, 4), # prompt, fixation, animation, stimuli |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more about how the do_inquiry
method is implemented in the displays. We will time-test this code 👍🏼
self.window.flip() | ||
ended_at = self.static_clock.getTime() | ||
# TODO: should we have a trigger for VEP_STIM_END? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we know the stim length
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do know the stim length and how long this should take, but the issue that I encountered is that rendering was possibly taking longer than this. I got the timing on my local machine to mostly within the allotted stim length by using the multi-polygon checkerboards, but there is always some variation and I haven't yet tested with the full alphabet. Another trigger here would allow us to confirm that all stim happened with the correct timing, though maybe this should only be used in the timing task.
If we need to make adjustments, there are a couple ways to resolve this:
- We could render fewer things: omit the static elements; use simpler checkerboard patterns such as 4x4 or 3x3, or use fewer boxes (4 instead of 6).
- We could do a time check after every frame to ensure that we're still within the stim length. However, this may cut the pattern short and the check adds a little overhead to each frame.
|
||
def validate(self): | ||
"""Validate invariants""" | ||
assert self.num_boxes == 4 or self.num_boxes == 6, 'Number of boxes must be 4 or 6' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's needed because it's possible to change these values between constructing the config and getting the positions. However, the code was repeated. I refactored to have the constructor use this method as well.
Overview
Modified VEP Display to support a configurable number of text areas with provided positions, as well as configurable VEP stimulus that can use any size of checkerboard pattern.
Ticket
https://www.pivotaltracker.com/story/show/184564386
Contributions
Test
Discussion
With just 4 text areas and a 2x2 checkerboard to represent each VEPStimuli, the flicker rate was able to render in the allotted 1 second. However, with 6 text areas, this increased the total number of stimuli to be rendered in each frame and the rendering of a single flicker took longer than 1 second. This problem was worse when the checkerboard size was increased to 3x3 or above. This is because each square on the checkerboard was represented using two shapes, a GratingStim for 'on' and one for 'off'. With 6 areas, this resulted in 48 visual shapes just to represent the stim.
I was able to reduce the number of stimuli by rendering each VEP Stim (checkerboard) as two visual shapes, a polygon with the primary color that has holes for each alternating square, and a backing square with the secondary color. On my local machine with 6 text boxes and 6 corresponding 5x5 checkerboards, the timing was able to complete in the allowed second. I put logging in place to confirm that this will work on other machines. If not, we can reduce the checkerboard size or draw fewer static elements.
vep_display.mov