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

Builder routine: routineTimer for nonSlip can get reset too early #822

Closed
jeremygray opened this Issue Feb 1, 2015 · 1 comment

Comments

Projects
None yet
1 participant
@jeremygray
Member

jeremygray commented Feb 1, 2015

This is not necessarily a bug, but it tripped me up, and I think at the very least we need to tell people more about this situation: when a nonSlip routine follows a non-nonSlip (dynamic length) routine.

In a dynamic routine, the global routineTimer gets .reset() if the while loop is not exiting:

if continueRoutine:  # don't flip if this routine is over or we'll get a blank screen
    win.flip()
else:  # this Routine was not non-slip safe so reset non-slip timer
    routineTimer.reset()

(Seems like that could just be done once, just after the while loop when it exits, not every frame?)

However, if you have a code component in that dynamic routine and it has code in End Routine, that code will get run after the loop has ended (= after routineTimer has been reset). the routineTimer then ticks away and time elapses. In my case this was during launchScan(), which waits for user input. If the next routine is nonSlip (in my case: fMRI trials), time is then added to the routineTimer. But any already-elapsed time (during launchScan) will count against the total, which is incorrect behavior. Fortunately I noticed by eye because it was seconds, but I would not have noticed even 200ms (e.g., to load a large file). Maybe I would have seen this in the log or data files, but it would be easily missed.

To fix, I ended up putting routineTimer.reset(); routineTimer.add(someTimeValue) in code at the start of the next (= nonSlip) routine, but I think it would be better just to do routineTimer.reset() in the code at the end of the previous (dynamic) routine.

If there's a way to handle this properly in code, I think we should do it.

If its too complicated to try to handle programmatically, we should document and warn people about it.

Are there any other cases where routineTimer could get off from the intended behavior?

@jeremygray

This comment has been minimized.

Member

jeremygray commented Feb 1, 2015

I suspect that the right thing to do is for routineTimer.reset() to happen as the very last thing upon completing a non-nonSlip (dynamic / unknown duration) routine, after any code in End Routine.

But I have not fully thought through the logic of nonSlip as implemented, however, so maybe I am missing something. I am unsure what happens (and what should happen) when chaining multiple nonSlip routines together. Lets say we have A, B, C that are all supposed to take 1.000s, all are nonSlip. If A ends up taking 1.100s (say 1.000s during the while loop and 0.1s for something after the while loop but before the next routine, like code in a code comp end routine, or the code to set up a loop), then the next routine B will start late (by 0.1s) but will end "on time" (after 2.000s). I think there's a warning given that A took longer than expected? I did not get such a warning. Maybe that's only for ISI periods.

jeremygray added a commit to jeremygray/psychopy that referenced this issue Feb 1, 2015

BF: when to reset the routineTimer; fixes psychopy#822
- handles my case (nonSlip follows non-nonSlip with code for launchScan in End Routine)
- might not fix all situations?

@peircej peircej closed this in d517c56 Feb 2, 2015

peircej added a commit that referenced this issue Feb 2, 2015

Merge pull request #825 from jeremygray/nonslip-reset-at-end-of-routine
BF: move routineTimer.reset() to end of routine; fixes #822

yarikoptic added a commit to neurodebian/psychopy that referenced this issue Apr 21, 2015

Merge tag '1.82.01' into releases
release 1.82.01

* tag '1.82.01': (28 commits)
  DOCS: changelog for and version release
  BF: MovieStim2 gave black frame on some macs (or some versions of OpenCV?)
  DOCS: mostly improvements to hardware (added BitsSharp)
  BF: Tobii calibration window not closing in 1.82
  Docs: start HTML title with current page
  BF: move routineTimer.reset() to end of routine; fixes psychopy#822
  ENH: 3 more reserved words for namespace
  DOC: Deleted mp3 support for sound
  BF: RatingScale always needs an origScaleDescription
  BF: stimuli sometimes don't appear after a texture has been created
  DOC: Fixed broken links
  DOC: Broken link from Builder to Coder Sound documentation
  Updated Tobii eye tracker iohub interface doc page
  BF: pygame sound did not have a loops parameter in __init__()
  ENH: update Japanese translation
  ENH: add translation to 'backend' in Movie Component
  TST: Add tests for filetools module
  RF/ENH: Extended filename collision handling
  RF/NF: Reduce code duplication
  BF: Don't disable auto-saving on manual save
  ...

yarikoptic added a commit to neurodebian/psychopy that referenced this issue Apr 21, 2015

Merge branch 'releases' into dfsg
* releases: (138 commits)
  DOCS: changelog for and version release
  BF: MovieStim2 gave black frame on some macs (or some versions of OpenCV?)
  DOCS: mostly improvements to hardware (added BitsSharp)
  BF: Tobii calibration window not closing in 1.82
  Docs: start HTML title with current page
  BF: move routineTimer.reset() to end of routine; fixes psychopy#822
  ENH: 3 more reserved words for namespace
  DOC: Deleted mp3 support for sound
  BF: RatingScale always needs an origScaleDescription
  BF: stimuli sometimes don't appear after a texture has been created
  DOC: Fixed broken links
  DOC: Broken link from Builder to Coder Sound documentation
  Updated Tobii eye tracker iohub interface doc page
  BF: pygame sound did not have a loops parameter in __init__()
  ENH: update Japanese translation
  ENH: add translation to 'backend' in Movie Component
  TST: Add tests for filetools module
  RF/ENH: Extended filename collision handling
  RF/NF: Reduce code duplication
  BF: Don't disable auto-saving on manual save
  ...

yarikoptic added a commit to neurodebian/psychopy that referenced this issue Apr 21, 2015

Merge tag '1.82.01.dfsg' into debian
DFSG version of 1.82.01

* tag '1.82.01.dfsg': (138 commits)
  DOCS: changelog for and version release
  BF: MovieStim2 gave black frame on some macs (or some versions of OpenCV?)
  DOCS: mostly improvements to hardware (added BitsSharp)
  BF: Tobii calibration window not closing in 1.82
  Docs: start HTML title with current page
  BF: move routineTimer.reset() to end of routine; fixes psychopy#822
  ENH: 3 more reserved words for namespace
  DOC: Deleted mp3 support for sound
  BF: RatingScale always needs an origScaleDescription
  BF: stimuli sometimes don't appear after a texture has been created
  DOC: Fixed broken links
  DOC: Broken link from Builder to Coder Sound documentation
  Updated Tobii eye tracker iohub interface doc page
  BF: pygame sound did not have a loops parameter in __init__()
  ENH: update Japanese translation
  ENH: add translation to 'backend' in Movie Component
  TST: Add tests for filetools module
  RF/ENH: Extended filename collision handling
  RF/NF: Reduce code duplication
  BF: Don't disable auto-saving on manual save
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment