Skip to content

Fixes to avoid data loss - #98

Draft
deruyter92 wants to merge 8 commits into
cy/extend-processor-apifrom
jaap/experimental-fixes
Draft

Fixes to avoid data loss#98
deruyter92 wants to merge 8 commits into
cy/extend-processor-apifrom
jaap/experimental-fixes

Conversation

@deruyter92

Copy link
Copy Markdown
Contributor

Summary
Two defensive fixes on the GUI side to prevent silently losing processor
outputs when the DLC processor shuts down before the recording-stopped
hook fires.

The custom processor plugin dlc_inference_w_pd_sync saves its legacy
outputs (PROC pickle, DLC HDF5, timestamp NPY) and performs DB-compatible
file copies through on_recording_stopped().

some non-standard ways of ending the experiment could bypass
this hook

  1. Any closeEvent: the shutdown skipped _cleanup_processor() when the worker thread
    stopped cleanly. After Fix thread crash in TeensyLatency + prevent data-loss for dlc_inference_w_pd_sync MMathisLab/FreelyMovingVR4Mice#329, the processor should now save it's own data when stopping. So cleanup of the processor is good here and safe.

  2. Stop inference while recording: _stop_inference() stops
    the processor via _dlc.reset() before the recording stops. When recording is later stopped,
    _notify_processor_recording_stopped() finds no processor instance.

changes:

  1. Add the missing self._cleanup_processor() call in the clean-shutdown
    branch of shutdown(), so that processor resources are properly released
    and (with the processor-side fix) data is saved before the window closes.
  2. When the user clicks "Stop inference" while recording is active, a
    QMessageBox.question dialog now appears warning that legacy output
    copies will be skipped and recommending to stop recording first. If the
    user proceeds anyway, the processor saves its core data through its own
    stop() path. No prompt appears when recording is not active.

@C-AchardC-Achard left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few minor formatting comments, thanks for the fixes!

Comment threaddlclivegui/gui/main_window.py Outdated
Comment threaddlclivegui/gui/main_window.py Outdated
Comment threaddlclivegui/services/dlc_processor.py
@C-Achard
C-Achard changed the base branch from cy/TEST-unity-proc to cy/extend-processor-apiAugust 10, 2026 14:03
@C-Achard
C-Achard changed the base branch from cy/extend-processor-api to cy/TEST-unity-procAugust 10, 2026 14:03
@C-Achard
C-Achard marked this pull request as draft August 10, 2026 14:03
@C-Achard

Copy link
Copy Markdown
Contributor

@deruyter92 These are all great fixes, I just need to rebase it on top of the current stack as the test/experimental fixes branch has diverged now, I marked it as draft but will take care of it now

@C-Achard
C-Achardforce-pushed the jaap/experimental-fixes branch from fec449b to 1d1f3ddCompareAugust 10, 2026 14:11
@C-Achard
C-Achard changed the base branch from cy/TEST-unity-proc to cy/extend-processor-apiAugust 10, 2026 14:12
@C-Achard
C-Achardforce-pushed the jaap/experimental-fixes branch from 1d1f3dd to 3881628CompareAugust 10, 2026 14:27
@deruyter92
deruyter92 marked this pull request as ready for review August 11, 2026 06:51
@deruyter92
deruyter92 marked this pull request as draft August 11, 2026 06:51
@C-Achard
C-Achardforce-pushed the jaap/experimental-fixes branch from 3881628 to 9f8abe8CompareAugust 11, 2026 08:58
@C-Achard
C-Achardforce-pushed the jaap/experimental-fixes branch from 9f8abe8 to f7ed0dcCompareAugust 11, 2026 09:24
@C-Achard
C-Achardforce-pushed the jaap/experimental-fixes branch from f7ed0dc to e4eacf7CompareAugust 11, 2026 11:56
deruyter92and others added 8 commits August 11, 2026 15:23
… during recording
When the user clicks "Stop pose inference" before "Stop recording",
the processor instance was destroyed by reset() without saving its
accumulated data. Later the recording stop flow would find no
processor instance and silently skip the save.
Now _stop_inference() saves processor data first if recording is
still active, so data is preserved regardless of stop-button order.
partial save for crash path should not be called when stopping inference.
`shutdown()` was skipping `_cleanup_processor()` when the worker thread
stopped cleanly, leaving the custom processor's resources unreleased and its buffered data unsaved.
This commit adds the missing `_cleanup_processor()` call before tearing down the
DLCLive instance.
Stopping the DLC processor during a recording skips the processor's
`on_recording_stopped` hook, which would normally handle legacy output
copies and DB-compatible file alignment. Show a confirmation dialog
when the user attempts to stop inference while recording is still
active, recommending they stop recording first.
Co-authored-by: Cyril Achard <cyril.achard@epfl.ch>
Co-authored-by: Cyril Achard <cyril.achard@epfl.ch>
@C-Achard
C-Achardforce-pushed the jaap/experimental-fixes branch from e4eacf7 to 1d5f26fCompareAugust 11, 2026 13:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@deruyter92@C-Achard