Make processors compatible for dlclive gui update - #317
Conversation
…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>
Current status
Misc
|
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.
3155fd3 to
3b64ba8
Compare
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.
|
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. |
…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>
…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>
There was a problem hiding this comment.
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_processorandPROCESSOR_*fields across custom processors, plusget_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.
…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>
| 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
FreelyMovingVR4Mice/dj_pipeline/vr4mice/actions/populate_rig.py
Lines 390 to 450 in 6695e3c
There was a problem hiding this comment.
yes this looks like a useful approach, thanks!
|
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 |
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, anddlc_inference_w_pd) now use the@register_processordecorator 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.pySocket-based processors (
MyProcessor_socketand its subclass) now implement robust connection management and resource cleanup. Connections are established only when needed, and newstop,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:
find_related_filesand 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 #327Bug fixed with racing case in #329.