Fix fixed target display intervals additional delay bug

- repeat of condition with `fixed target display intervals` now continues the same fixed intervals - previously the time from the last target of the previous trial was not taken into account - this meant the first target of each repeat had a longer delay than desired - store the time used by most recent target for current / previous trial in `TaskManager` to allow this - extend `test_task_fixed_intervals_no_user_input` to include this case in tests - bump version to 1.3.2 - update changelog - add `gnome-screenshot` to linux CI deps - resolves #256
ssciwr · Dec 8, 2023 · e16f68b · e16f68b
1 parent 96270c0
commit e16f68b
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 22 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -27,7 +27,7 @@ jobs:
         include:
           - os: "ubuntu-22.04"
             python-version: "3.10"
-            psychopy-version: "2023.2.2"
+            psychopy-version: "2023.2.3"
           - os: "ubuntu-22.04"
             python-version: "3.10"
             psychopy-version: "2023.1.3"
@@ -50,7 +50,7 @@ jobs:
         if: runner.os == 'Linux'
         run: |
           # various psychopy & qt system dependencies
-          sudo apt-get update -yy && sudo apt-get install -yy libasound2-dev portaudio19-dev libpulse-dev libusb-1.0-0-dev libsndfile1-dev libportmidi-dev liblo-dev libsdl2-mixer-2.0-0 libsdl2-image-2.0-0 libsdl2-2.0-0 freeglut3-dev scrot libnotify-dev pandoc libglu1-mesa-dev libx11-dev libx11-xcb-dev libxext-dev libxfixes-dev libxi-dev libxrender-dev libxcb1-dev libxcb-glx0-dev libxcb-keysyms1-dev libxcb-image0-dev libxcb-shm0-dev libxcb-icccm4-dev libxcb-sync-dev libxcb-xfixes0-dev libxcb-shape0-dev libxcb-randr0-dev libxcb-render-util0-dev libxkbcommon-dev libxkbcommon-x11-dev '^libxcb.*-dev'
+          sudo apt-get update -yy && sudo apt-get install -yy libasound2-dev portaudio19-dev libpulse-dev libusb-1.0-0-dev libsndfile1-dev libportmidi-dev liblo-dev libsdl2-mixer-2.0-0 libsdl2-image-2.0-0 libsdl2-2.0-0 freeglut3-dev scrot libnotify-dev pandoc libglu1-mesa-dev libx11-dev libx11-xcb-dev libxext-dev libxfixes-dev libxi-dev libxrender-dev libxcb1-dev libxcb-glx0-dev libxcb-keysyms1-dev libxcb-image0-dev libxcb-shm0-dev libxcb-icccm4-dev libxcb-sync-dev libxcb-xfixes0-dev libxcb-shape0-dev libxcb-randr0-dev libxcb-render-util0-dev libxkbcommon-dev libxkbcommon-x11-dev '^libxcb.*-dev' gnome-screenshot
           # install pre-built ubuntu/gtk3 wxPython wheel
           pip install -f https://extras.wxpython.org/wxPython4/extras/linux/gtk3/${{ matrix.os }}/ wxPython
           # enable colours in logs

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,17 @@
 # Changelog
 
+## [1.3.2] - 2023-12-08
+
+### Fixed
+
+- additional delay between repetitions of a condition when using fixed target display intervals [#256](https://github.com/ssciwr/vstt/issues/256)
+
+## [1.3.1] - 2023-11-06
+
+### Fixed
+
+- missing dependency for vstt 1.3.0 [#254](https://github.com/ssciwr/vstt/pull/254)
+
 ## [1.3.0] - 2023-09-29
 
 ### Added

diff --git a/src/vstt/__init__.py b/src/vstt/__init__.py
@@ -5,4 +5,4 @@
     "__version__",
 ]
 
-__version__ = "1.3.1"
+__version__ = "1.3.2"
diff --git a/src/vstt/task.py b/src/vstt/task.py
@@ -97,6 +97,8 @@ def __init__(self, win: Window, trial: vstt.vtypes.Trial):
         if trial["show_cursor_path"]:
             self.drawables.append(self._cursor_path)
         self.first_target_of_condition_shown = False
+        self.most_recent_target_display_time = 0.0
+        self.final_target_display_time_previous_trial = 0.0
 
     def cursor_path_add_vertex(
         self, vertex: Tuple[float, float], clear_existing: bool = False
@@ -233,6 +235,9 @@ def _do_trial(
         if trial["use_joystick"] and self.js is None:
             raise RuntimeError("Use joystick option is enabled, but no joystick found.")
         trial_manager.cursor.setPos(initial_cursor_pos)
+        trial_manager.final_target_display_time_previous_trial = (
+            trial_manager.most_recent_target_display_time
+        )
         trial_data = TrialData(trial, self.rng)
         self.win.recordFrameIntervals = True
         trial_manager.clock.reset()
@@ -282,7 +287,9 @@ def _do_target(
         stop_target_time = 0.0
         if trial["fixed_target_intervals"]:
             num_completed_targets = len(trial_data.to_target_timestamps)
-            stop_waiting_time = (num_completed_targets + 1) * trial["target_duration"]
+            stop_waiting_time = (num_completed_targets + 1) * trial[
+                "target_duration"
+            ] - tm.final_target_display_time_previous_trial
             stop_target_time = stop_waiting_time + trial["target_duration"]
         for target_index in _get_target_indices(index, trial):
             t0 = tm.clock.getTime()
@@ -398,6 +405,7 @@ def _do_target(
                     dist > target_size
                     and tm.clock.getTime() + minimum_window_for_flip < stop_target_time
                 )
+            tm.most_recent_target_display_time = tm.clock.getTime() - stop_waiting_time
             success = (
                 dist_correct <= target_size
                 and tm.clock.getTime() + minimum_window_for_flip < stop_target_time

diff --git a/tests/test_task.py b/tests/test_task.py
@@ -172,6 +172,7 @@ def test_task_fixed_intervals_no_user_input(window: Window) -> None:
     experiment.metadata["display_duration"] = 0.0
     target_duration = 1.0
     trial = vstt.trial.default_trial()
+    trial["weight"] = 3
     trial["num_targets"] = 3
     trial["target_order"] = "random"
     trial["show_target_labels"] = True
@@ -192,30 +193,41 @@ def test_task_fixed_intervals_no_user_input(window: Window) -> None:
     # check that we failed to hit all targets
     expected_success = np.full((trial["num_targets"],), False)
     data = experiment.trial_handler_with_results.data
-    for success_name in ["to_target_success", "to_center_success"]:
-        assert np.all(data[success_name][0][0] == expected_success)
-    # first to_target timestamps should start at ~0,
-    # at ~target_duration the first target is displayed for target_duration secs,
-    # subsequent ones should be displayed every ~target_duration starting from ~2*target_duration
-    all_to_target_timestamps = data["to_target_timestamps"][0][0]
-    # require timestamps to be accurate within 0.5s
-    allowed_error_on_timestamp = 0.5
-    # this is a weak requirement to avoid tests failing on CI where many frames can get dropped
-    # if running tests locally allowed_error_on_timestamp should be ~2/fps
-    assert abs(all_to_target_timestamps[0][0]) < allowed_error_on_timestamp
-    assert (
-        abs(all_to_target_timestamps[0][-1] - 2 * target_duration)
-        < allowed_error_on_timestamp
-    )
-    for count, to_target_timestamps in enumerate(all_to_target_timestamps[1:]):
+    assert data["to_target_timestamps"].shape == (3, 1)
+    for irep in [0, 1, 2]:
+        for success_name in ["to_target_success", "to_center_success"]:
+            assert np.all(data[success_name][irep][0] == expected_success)
+        # For first target of first repetition: to_target timestamps should go from 0 to 2*target_duration
+        # All subsequent to_target timestamps should follow sequentially and last target_duration
+        # Each rep resets the clock to zero
+        all_to_target_timestamps = data["to_target_timestamps"][irep][0]
+        # require timestamps to be accurate within 0.5s
+        allowed_error_on_timestamp = 0.5  # 1.0/30.0
+        # this is a weak requirement to avoid tests failing on CI where many frames can get dropped
+        # if running tests locally allowed_error_on_timestamp should be ~2/fps
+        expected_initial_t_first_target = 0
+        expected_final_t_first_target = (
+            2 * target_duration if irep == 0 else target_duration
+        )
         assert (
-            abs(to_target_timestamps[0] - (count + 2) * target_duration)
+            abs(all_to_target_timestamps[0][0]) - expected_initial_t_first_target
             < allowed_error_on_timestamp
         )
         assert (
-            abs(to_target_timestamps[-1] - (count + 3) * target_duration)
+            abs(all_to_target_timestamps[0][-1] - expected_final_t_first_target)
             < allowed_error_on_timestamp
         )
+        for count, to_target_timestamps in enumerate(all_to_target_timestamps[1:]):
+            expected_initial_t = expected_final_t_first_target + count * target_duration
+            expected_final_t = expected_initial_t + target_duration
+            assert (
+                abs(to_target_timestamps[0] - expected_initial_t)
+                < allowed_error_on_timestamp
+            )
+            assert (
+                abs(to_target_timestamps[-1] - expected_final_t)
+                < allowed_error_on_timestamp
+            )
 
 
 def test_task_condition_timeout_no_user_input(window: Window) -> None: