Skip to content

Make processors compatible for dlclive gui update - #317

Open
CeliaBenquet wants to merge 33 commits into
mainfrom
celia/register-processors
Open

Make processors compatible for dlclive gui update#317
CeliaBenquet wants to merge 33 commits into
mainfrom
celia/register-processors

Conversation

@CeliaBenquet

@CeliaBenquet CeliaBenquet commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator
  • Check compatibility with previous GUI???

Main changes include enhanced processor registration and metadata, robust resource cleanup for socket-based processors, and improved session file matching logic for multi-camera rigs.

Processor registration, metadata, and resource management:

  • All custom processors (dlc_only, MyProcessor_socket, and dlc_inference_w_pd) now use the @register_processor decorator and are added to a central registry for discovery and configuration for the DLC live GUI 2.0. Each processor defines descriptive metadata and parameter specifications for GUI integration and programmatic use. (mouse_task/dlc_utils/dlcProcessor_dlconly.py

  • Socket-based processors (MyProcessor_socket and its subclass) now implement robust connection management and resource cleanup. Connections are established only when needed, and new stop, close, and resource-release methods ensure sockets and listeners are properly closed, preventing resource leaks. (mouse_task/dlc_utils/dlc_processor_socket.py)

Multi-camera file matching improvements:

  • The session file matching logic (find_related_files and helpers) in the transfer module now respects camera indices for multi-camera rigs. When a user picks a file with a camera number, only sibling files from the same camera are auto-filled, preventing mismatches. If no camera index is present, a sensible default (camera 3) is used or, if ambiguous, no auto-fill occurs. (dj_pipeline/gui_transfer/utils/session_files.py) from Improve compatibility to multiple cameras for autocompletion of the paths in transfer gui #327

Bug fixed with racing case in #329.

CeliaBenquet and others added 3 commits July 3, 2026 10:31
…319)

* feat: add modular task config files and loader

Add a common.yaml holding every task parameter's default, plus one
tasks/<name>.yaml per variant declaring only the entries that differ
(along with its class_name and description).

helpers.load_task_config(<name>) deep-merges the common defaults with a
task's overrides, replacing the duplicated per-task parameter sets that
previously lived in separate Python modules. Includes a config for
far_reward_mouse_detection_p2, the one task variant not otherwise covered
by an existing tasks/*.yaml.

* feat: make ActiveSensingTask config-driven and generate task classes

ActiveSensingTask now accepts a task_config argument selecting a task
YAML; parameters resolve from the merged config, with any explicitly
passed argument taking precedence. Missing parameters (no task_config and
not passed) raise a clear error.

_registry builds one ActiveSensingTask subclass per task YAML at import
and exposes them as mouse_task.<ClassName>. The generated __init__ keeps a
real, introspectable signature (every parameter as a keyword arg with its
merged default) so the teensyexp GUI can still build its editable
parameter form.

* refactor: remove per-task subclass modules

The 20 inv_*/mouse_*/shape_*/far_reward_* modules were near-identical
copies differing only in parameter values, which now live in
configs/tasks/*.yaml and are materialised as classes by the registry.

* test: cover task-config merge, registry, and generated signatures

Add tests verifying every task YAML yields an exposed ActiveSensingTask
subclass, load_task_config merges common defaults with task overrides, the
generated __init__ signatures expose every parameter with its merged
default, and constructing a task resolves parameters correctly (explicit
arguments overriding the config).

* docs: describe the modular task-config registry

Repoint links to the YAML configs and document how task variants are now
defined (common.yaml + tasks/*.yaml) and materialised as classes by the
registry, replacing the per-task module description.

* Fix the modular task set up by testing on the rig  (#320)

* Fix task modularization

* Refine further reward port distance based on rig

---------

Co-authored-by: Célia Benquet <32598028+CeliaBenquet@users.noreply.github.com>
@C-Achard

C-Achard commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Current status

  • Fix Unity/Teensy <> Custom processor <> DLC-live-GUI communication
  • Re-introduce previous file outputs from previous GUI
  • Ensure file format, name and content conventions match legacy data
  • Confirm full experimental pipeline is OK, including database export step
  • [~] Troubleshoot PyTorch models performance

Misc

  • Fix some UI fields having missing or incorrect persistence

@CeliaBenquet
CeliaBenquet changed the base branch from main to celia/further-reward-ports July 13, 2026 07:34
CeliaBenquet and others added 12 commits July 13, 2026 09:35
Wrap processor initialization in a guarded try/except so partial startup failures trigger cleanup and re-raise as a clear RuntimeError. Add a dedicated stop() method (plus close() alias) to optionally save output and reliably release Teensy, socket connection, and listener resources, reducing leaked serial handles/ports after errors or shutdown.
Implement a dedicated `save()` method in `dlc_inference_w_pd_sync` that persists latency data to a pickle file, supports an optional explicit path, and falls back to `self.save_path` when no file is passed. The change also initializes `save_path` on construction, ensures parent directories are created, and adds warning-based error handling for missing paths or save failures.
Replaced `import datetime` with `from datetime import datetime` in `dlc_processor_socket_pd_sync.py` to align the import with direct `datetime` usage and avoid module/class ambiguity.
Add a `_video_prefix()` helper that derives the base name from discovered video files, falling back to `filename_stem` and then `recording`. Update `_db_compat_base()` to use this prefix instead of the mouse identifier when building the output path, improving alignment with recording/video naming.
C-Achard and others added 3 commits July 13, 2026 10:47
Add `_select_single_pose` to normalize incoming pose arrays before processing. The method now accepts both `(K, 3)` and `(N, K, 3)` inputs and, when multiple detections are present, picks the one with the highest mean keypoint likelihood (with a warning fallback when scores are non-finite). `process()` now feeds this selected single pose into the parent processor to prevent shape mismatches while keeping legacy buffering behavior.
@CeliaBenquet

CeliaBenquet commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

related branch on DeepLabCut-live-GUI, https://github.com/DeepLabCut/DeepLabCut-live-GUI/tree/cy/TEST-unity-proc.

Now works for Tensorflow models, and used on latest experiments.

Base automatically changed from celia/further-reward-ports to main July 22, 2026 17:02
…aths in transfer gui (#327)

* Make processors compatible for dlclive gui update

* Update dlcliveonly

* Add HEAD_CONF_THRESHOLD to MyProcessor_socket and dlc_inference_w_pd_sync classes

* Harden PD sync processor initialization cleanup

Wrap processor initialization in a guarded try/except so partial startup failures trigger cleanup and re-raise as a clear RuntimeError. Add a dedicated stop() method (plus close() alias) to optionally save output and reliably release Teensy, socket connection, and listener resources, reducing leaked serial handles/ports after errors or shutdown.

* Add default-path save for PD sync processor

Implement a dedicated `save()` method in `dlc_inference_w_pd_sync` that persists latency data to a pickle file, supports an optional explicit path, and falls back to `self.save_path` when no file is passed. The change also initializes `save_path` on construction, ensures parent directories are created, and adds warning-based error handling for missing paths or save failures.

* Enhance dlc_inference_w_pd_sync with legacy recording support and timestamp handling

* Refactor dlc_inference_w_pd_sync for improved legacy support and enhanced logging

* Refactor dlc_inference_w_pd_sync for DB compatibility and improved timestamp handling

* Use direct datetime import in DLC sync

Replaced `import datetime` with `from datetime import datetime` in `dlc_processor_socket_pd_sync.py` to align the import with direct `datetime` usage and avoid module/class ambiguity.

* Use video prefix in DB compat base path

Add a `_video_prefix()` helper that derives the base name from discovered video files, falling back to `filename_stem` and then `recording`. Update `_db_compat_base()` to use this prefix instead of the mouse identifier when building the output path, improving alignment with recording/video naming.

* Update dlc_processor_socket_pd_sync.py

* Update dlc_processor_socket_pd_sync.py

* Enhance multi-camera support by extracting camera index from filenames and updating related file fetching logic

* Implement review comments

---------

Co-authored-by: Cyril Achard <cyril.achard@epfl.ch>
@CeliaBenquet
CeliaBenquet requested a review from deruyter92 August 4, 2026 08:34
CeliaBenquet and others added 2 commits August 4, 2026 10:38
…rence_w_pd_sync` (#329)

* fix `TeensyLatency`: join reader thread before closing serial port

`close_serial()` was calling `self.ser.close()` immediately after
signalling `_stop_reading()`, without waiting for the `read_on_thread`
to exit its loop.

* fix dlc_inference_w_pd_sync: save before cleanup when stopping active recording.

it only saved data through the
`on_recording_stopped` hook, which runs after the GUI's recording
manager finalizes video files. This commit overrides `stop()` to save all three legacy outputs and clear buffers before cleanup.

The save is guarded so it only runs when a recording was active and a
`save_path` has been set. (the crash path, not the happy path)

* add calls to renaming function in `dlc_inference_w_pd_sync.stop()`

These were present in `on_recording_stopped` but not in `stop`

* add centralized `_save_legacy_outputs` helper to deduplicate `on_recording_stopped` and the `stop` override method.

* add missing timeout to `serial.Serial` call

* TeensyLatency: skip empty reads to avoid wasted decode/parse cycles when idle

---------

Co-authored-by: Célia Benquet <32598028+CeliaBenquet@users.noreply.github.com>
@CeliaBenquet CeliaBenquet added enhancement New feature or request and removed don't merge labels Aug 4, 2026
@CeliaBenquet
CeliaBenquet requested a lite review from Copilot August 4, 2026 08:46

Copilot AI 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.

Pull request overview

This PR updates several DLC Live “processor” plugins to work with DLC Live GUI 2.0 discovery/metadata, improves lifecycle cleanup for socket-based processors, and refines transfer-GUI session file autocompletion to avoid multi-camera mismatches.

Changes:

  • Added processor registration/metadata via @register_processor and PROCESSOR_* fields across custom processors, plus get_available_processors() helpers.
  • Improved socket/Teensy resource management (deferred socket accept, explicit stop/close, safer serial read thread shutdown).
  • Updated transfer GUI session file matching to respect camera indices, with new unit tests covering multi-camera behaviors.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/unit/test_gui_transfer.py Adds unit tests for camera-number parsing and multi-camera sibling-file matching.
mouse_task/latency_tests/Teensy_latency/TeensyLatency.py Makes serial reads timeout-based and ensures reader thread is joined before closing.
mouse_task/dlc_utils/simple_processor.py Registers TeensyLaser with GUI processor registry and adds metadata/getter.
mouse_task/dlc_utils/dlcProcessor_dlconly.py Registers dlc_only with GUI processor registry and adds metadata/getter.
mouse_task/dlc_utils/dlc_processor_socket.py Refactors socket processor to accept connections lazily and adds cleanup hooks/metadata.
mouse_task/dlc_utils/dlc_processor_socket_pd.py Adds GUI registration/metadata and safer optional import handling for Teensy dependency.
mouse_task/dlc_utils/dlc_processor_socket_pd_sync.py Major expansion to support legacy outputs + GUI hooks, registration, and cleanup.
dj_pipeline/gui_transfer/utils/session_files.py Adds camera-index extraction and constrains sibling-file matching by camera.
dj_pipeline/gui_transfer/modules/transfer.py Passes selected file’s camera number into find_related_files for correct autocompletion.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread mouse_task/dlc_utils/simple_processor.py
Comment thread mouse_task/dlc_utils/dlcProcessor_dlconly.py
Comment thread mouse_task/dlc_utils/dlc_processor_socket.py
Comment thread mouse_task/dlc_utils/dlc_processor_socket_pd.py
Comment thread mouse_task/dlc_utils/dlc_processor_socket_pd_sync.py
Comment thread mouse_task/dlc_utils/dlc_processor_socket.py Outdated
Comment thread mouse_task/dlc_utils/dlc_processor_socket_pd_sync.py Outdated
CeliaBenquet and others added 6 commits August 5, 2026 15:13
…older (#337)

* fix `TeensyLatency`: join reader thread before closing serial port

`close_serial()` was calling `self.ser.close()` immediately after
signalling `_stop_reading()`, without waiting for the `read_on_thread`
to exit its loop.

* fix dlc_inference_w_pd_sync: save before cleanup when stopping active recording.

it only saved data through the
`on_recording_stopped` hook, which runs after the GUI's recording
manager finalizes video files. This commit overrides `stop()` to save all three legacy outputs and clear buffers before cleanup.

The save is guarded so it only runs when a recording was active and a
`save_path` has been set. (the crash path, not the happy path)

* add calls to renaming function in `dlc_inference_w_pd_sync.stop()`

These were present in `on_recording_stopped` but not in `stop`

* add centralized `_save_legacy_outputs` helper to deduplicate `on_recording_stopped` and the `stop` override method.

* add missing timeout to `serial.Serial` call

* TeensyLatency: skip empty reads to avoid wasted decode/parse cycles when idle

* Improve gui transfer of processed files

* Clarify documentation for remote path handling and improve conditional checks in Transfer class

---------

Co-authored-by: Jaap de Ruyter van Steveninck <32810691+deruyter92@users.noreply.github.com>
* Make processors compatible for dlclive gui update

* Update dlcliveonly

* Add HEAD_CONF_THRESHOLD to MyProcessor_socket and dlc_inference_w_pd_sync classes

* Harden PD sync processor initialization cleanup

Wrap processor initialization in a guarded try/except so partial startup failures trigger cleanup and re-raise as a clear RuntimeError. Add a dedicated stop() method (plus close() alias) to optionally save output and reliably release Teensy, socket connection, and listener resources, reducing leaked serial handles/ports after errors or shutdown.

* Add default-path save for PD sync processor

Implement a dedicated `save()` method in `dlc_inference_w_pd_sync` that persists latency data to a pickle file, supports an optional explicit path, and falls back to `self.save_path` when no file is passed. The change also initializes `save_path` on construction, ensures parent directories are created, and adds warning-based error handling for missing paths or save failures.

* Enhance dlc_inference_w_pd_sync with legacy recording support and timestamp handling

* Refactor dlc_inference_w_pd_sync for improved legacy support and enhanced logging

* Refactor dlc_inference_w_pd_sync for DB compatibility and improved timestamp handling

* Use direct datetime import in DLC sync

Replaced `import datetime` with `from datetime import datetime` in `dlc_processor_socket_pd_sync.py` to align the import with direct `datetime` usage and avoid module/class ambiguity.

* Use video prefix in DB compat base path

Add a `_video_prefix()` helper that derives the base name from discovered video files, falling back to `filename_stem` and then `recording`. Update `_db_compat_base()` to use this prefix instead of the mouse identifier when building the output path, improving alignment with recording/video naming.

* Update dlc_processor_socket_pd_sync.py

* Update dlc_processor_socket_pd_sync.py

* Enhance multi-camera support by extracting camera index from filenames and updating related file fetching logic

* Improve Teensy serial reading with error handling and timeout

* Refactor code for improved readability and error handling across multiple files

* Remove autosave

* Add warning on new session launching in vr4mice gui

* Remove tmp files step

* Add experiment lifecycle documentation to the developer guide

* Copy PR#329 implementation

* Make ctrl c safe if data not saved

* Refactor DLC client to ensure proper socket closure

* Udpate docs

* Add tests for Teensy experiment GUI close behavior and latency exception handling

* Add unit tests for UnityTask to simulate environment interactions

* Refactor DLCClient to use socket connections with timeout handling and add regression tests for Teensy latency exceptions

* Restart warning blocks action

---------

Signed-off-by: Célia Benquet <32598028+CeliaBenquet@users.noreply.github.com>
Co-authored-by: Cyril Achard <cyril.achard@epfl.ch>
Comment on lines +55 to +63
def build_session_stem(
mouse: str,
date: str,
attempt: str,
namespace: str | None = None,
) -> str:
"""Build <mouse>_<date>_<attempt>, optionally with a namespace prefix."""
stem = f"{mouse}_{date}_{attempt}"
return f"{namespace}_{stem}" if namespace else stem

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good addition.

Non-blocking note:
Is this the file we want to put this though?

I am not entirely aware of all the differences between dlc_processor_socket, dlc_processor_socket_pd and dlc_processor_socket_pd_sync; If this is something you foresee that will be useful in different code branches and not only the current module, we should make it a general helper I think. This can help to avoid (more) code duplication.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If it's up to me, feel free to delegate this to future refactors so you focus on doing your experiments now. Maybe we can support with refactoring / making the codebase more robust for long-term after merging this PR.

@C-Achard C-Achard Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just a late side note, I think the discussion was also to change the parsing in the DB-populating GUI, to be either up to date with new output formats or have more flexible parsing.

For reference:

def get_files_paths(
dataset,
remote_src: Optional[str] = None,
local_src: str = "/data",
data: str = "/data",
filename: str = os.environ["IMG_SRC"],
):
"""
Simulation of data from gui .npy, if it's missing
Args:
dataset: The name of the dataset, formatting is {mouse_name}-{doe}-{attempt}.
remote_src: The source path for remote files.
local_src: The source path for local files.
data: The data path.
filename: The base filename for the video files.
"""
dlc_video_path = local_src + "/dlc_video"
files_info = {
"teensy_path": {
"filename": dataset + ".pickle",
"src": remote_src,
"dst": local_src + data,
},
"dlc_path": {
"filename": filename + "_" + dataset + "_DLC.hdf5",
"src": remote_src,
"dst": dlc_video_path,
},
"camera_path": {
"filename": filename + "_" + dataset + "_TS.npy",
"src": remote_src,
"dst": dlc_video_path,
},
"video_path": {
"filename": filename + "_" + dataset + "_VIDEO.avi",
"src": filename + "_" + dataset + "_VIDEO.avi",
"dst": dlc_video_path, # false (remote only)
},
"proc_path": {
"filename": filename + "_" + dataset + "_PROC",
"src": remote_src,
"dst": dlc_video_path,
},
"gui_output": {
"filename": dataset + ".npy",
"src": remote_src,
"dst": local_src + data,
},
"video_meta": {"duration": None, "fps": None, "width": None, "height": None},
"screen_recording_output": {
"filename": dataset + ".mkv",
"dst": "/vr4mice_screen_recordings/raw_screen_recordings/",
},
"time_stamp": None,
"doe": parse_date(dataset),
"dataset": dataset,
}
return files_info

@deruyter92 deruyter92 Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

yes this looks like a useful approach, thanks!

@deruyter92

Copy link
Copy Markdown
Collaborator

BTW, did you check the compatibility with previous GUI? Maybe we should add the DLC-live-GUI as (pinned) optional dependency to the setup.cfg

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants