-
Notifications
You must be signed in to change notification settings - Fork 54
[WIP] Continue work on Integration with DeepLabCut 3.0 #127
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
Conversation
- changed benchmarking link; needs testing
* Update .gitignore * CI/CD update python version * CI/CD update actions versions * CI/CD update trigger events * CI/CD update MacOS version * dlclibrary set version to >=0.0.6 * Poetry lock * Poetry lock * Pyproject.toml update tensorflow installation * Poetry lock * Install specific tensorflow-io-gcs-filesystem for windows * Poetry lock * Update deprecated section name * Poetry lock * CI/CD test on python 3.11, 3.12, 3.13 as well * Update testing.yml - rename to main - add tables installation --------- Co-authored-by: Mackenzie Mathis <mathis@rowland.harvard.edu>
* Update README.md fix test instructions * Update check_install.py * Update check_install.py * Update check_install.py
* Update pyproject.toml - adding support for python 3.11, 3.12, 3.13 * Update testing.yml * Update testing.yml * Update pyproject.toml * Update testing.yml * Update pyproject.toml * Update poetry.lock * Update poetry.lock * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update testing.yml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update testing.yml * Update pyproject.toml * Update pyproject.toml * Update poetry.lock * Update testing.yml * Update testing.yml * Update testing.yml - remove poetry (for now) * Update testing.yml * Update testing.yml * Update testing.yml * Update testing.yml * Update testing.yml * updated poetry.lock * updated * Update testing.yml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update testing.yml testing 3.11 * Update testing.yml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update testing.yml * updated test for win32 * Update testing.yml * Update testing.yml * Update testing.yml * Update testing.yml
0cc7137 to
7599121
Compare
b3a7ae8 to
db6a61d
Compare
e9177a9 to
09116e3
Compare
09116e3 to
e994808
Compare
There was a problem hiding this comment.
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 continues the integration of DeepLabCut 3.0 with PyTorch support, merging the main branch and consolidating three separate benchmarking modules into a single unified module that works with both TensorFlow and PyTorch backends.
Changes:
- Merged
benchmark_tf.pyandbenchmark_pytorch.pyinto a singlebenchmark.pymodule supporting both engines - Added
Engineenum class to detect and classify model types (TensorFlow vs PyTorch) - Updated factory pattern and check_install scripts to use the new Engine detection
- Fixed PyTorch runner bug where empty detection batches caused issues
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 27 comments.
Show a summary per file
| File | Description |
|---|---|
| dlclive/engine.py | New Engine enum for TensorFlow/PyTorch model type detection |
| dlclive/benchmark.py | Unified benchmarking module supporting both TF and PyTorch |
| dlclive/benchmark_tf.py | Removed - merged into benchmark.py |
| dlclive/benchmark_pytorch.py | Removed - merged into benchmark.py |
| dlclive/factory.py | Updated to use Engine enum for model type detection |
| dlclive/pose_estimation_pytorch/runner.py | Fixed empty detection batch handling |
| dlclive/check_install/check_install.py | Updated to support both engine types |
| dlclive/init.py | Added exports for benchmark functions |
| tests/test_benchmark_script.py | New functional test for benchmarking |
| pytest.ini | Added pytest configuration with functional marker |
| pyproject.toml | Updated dependencies, moved opencv-python-headless, added pytest |
| benchmarking/run_dlclive_benchmark.py | Updated to use Engine detection |
| README.md | Updated test instructions |
| .gitignore | Added __MACOSX exclusion |
| .github/workflows/testing.yml | Updated CI to support conda and both engines |
| .github/workflows/python-package.yml | Updated branch references from master to main |
Comments suppressed due to low confidence (2)
dlclive/benchmark.py:868
- Keyword argument 'precision' is not a supported parameter name of function benchmark_videos.
Keyword argument 'save_dir' is not a supported parameter name of function benchmark_videos.
Keyword argument 'device' is not a supported parameter name of function benchmark_videos.
Keyword argument 'draw_keypoint_names' is not a supported parameter name of function benchmark_videos.
Keyword argument 'get_sys_info' is not a supported parameter name of function benchmark_videos.
benchmark_videos(
video_path=args.video_path,
model_path=args.model_path,
model_type=args.model_type,
device=args.device,
precision=args.precision,
display=args.display,
pcutoff=args.pcutoff,
display_radius=args.display_radius,
resize=tuple(args.resize) if args.resize else None,
cropping=args.cropping,
dynamic=tuple(args.dynamic),
save_poses=args.save_poses,
save_dir=args.save_dir,
draw_keypoint_names=args.draw_keypoint_names,
cmap=args.cmap,
get_sys_info=args.get_sys_info,
save_video=args.save_video,
)
dlclive/check_install/check_install.py:13
- This import of module urllib.request is redundant, as it was previously imported on line 9.
import urllib.request
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pandas = ">=1.0.1,!=1.5.0" | ||
| tables = "^3.8" | ||
| opencv-python-headless = "^4.5" | ||
| pytest = "^8.0" |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding pytest as a regular dependency (not a dev dependency) is unusual. Test frameworks are typically dev dependencies. Consider moving pytest to [tool.poetry.group.dev.dependencies] instead.
| dog_models = glob.glob(str(datafolder / "dog" / "*[!avi]")) | ||
| dog_video = glob.glob(str(datafolder / "dog" / "*.avi"))[0] | ||
| mouse_models = glob.glob(str(datafolder / "mouse_lick" / "*[!avi]")) |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The glob pattern "*[!avi]" doesn't correctly exclude .avi files. This pattern matches any path that doesn't end with 'i' (just the last character). To properly exclude .avi files, use a more explicit approach like filtering the results or using a different pattern like "*/[!.]*.pb" for TensorFlow models or "*/*.pt" for PyTorch models.
| dog_models = glob.glob(str(datafolder / "dog" / "*[!avi]")) | |
| dog_video = glob.glob(str(datafolder / "dog" / "*.avi"))[0] | |
| mouse_models = glob.glob(str(datafolder / "mouse_lick" / "*[!avi]")) | |
| dog_models = [p for p in glob.glob(str(datafolder / "dog" / "*")) if not p.lower().endswith(".avi")] | |
| dog_video = glob.glob(str(datafolder / "dog" / "*.avi"))[0] | |
| mouse_models = [p for p in glob.glob(str(datafolder / "mouse_lick" / "*")) if not p.lower().endswith(".avi")] |
| result = benchmark_videos( | ||
| model_path=model_path, | ||
| model_type="base" if Engine.from_model_path(model_path) == Engine.TENSORFLOW else "pytorch", | ||
| video_path=dog_video, | ||
| output=str(out_dir), | ||
| n_frames=n_frames, | ||
| pixels=pixels | ||
| ) | ||
| print("Dog model result:", result) |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The benchmark_videos() function doesn't return a value according to its implementation (it only saves results to disk when output is provided), but the test stores and prints the return value. This could lead to confusion. Either remove the assignment and print statements, or check the actual output files instead.
|
|
||
| # make temporary directory in $HOME | ||
| # TODO: why create this temp directory in $HOME? | ||
| # make temporary directory in $current |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment says "make temporary directory in $current" but the code actually creates it in the home directory (Path().home() / "dlc-live-tmp"). Update the comment to accurately reflect where the directory is created, or change the code to match the comment.
| # make temporary directory in $current | |
| # make temporary directory in the user's home directory |
| video_path = video_path if type(video_path) is list else [video_path] | ||
|
|
||
| # fix resize | ||
| if pixels: | ||
| pixels = pixels if type(pixels) is list else [pixels] | ||
| resize = [None for p in pixels] | ||
| elif resize: | ||
| resize = resize if type(resize) is list else [resize] |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using type(pixels) is list and type(resize) is list are not recommended Python practices. Use isinstance() instead for type checking.
| video_path = video_path if type(video_path) is list else [video_path] | |
| # fix resize | |
| if pixels: | |
| pixels = pixels if type(pixels) is list else [pixels] | |
| resize = [None for p in pixels] | |
| elif resize: | |
| resize = resize if type(resize) is list else [resize] | |
| video_path = video_path if isinstance(video_path, list) else [video_path] | |
| # fix resize | |
| if pixels: | |
| pixels = pixels if isinstance(pixels, list) else [pixels] | |
| resize = [None for p in pixels] | |
| elif resize: | |
| resize = resize if isinstance(resize, list) else [resize] |
| @@ -0,0 +1,48 @@ | |||
| import os | |||
| import glob | |||
| import pathlib | |||
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'pathlib' is not used.
| import pathlib |
| """ | ||
| Downloads and extracts DeepLabCut-Live benchmarking data (videos & DLC models). | ||
| """ | ||
| import os |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import of module os is redundant, as it was previously imported on line 757.
| import os |
| except: | ||
| fourcc = "" | ||
| try: | ||
| fps = round(cap.get(cv2.CAP_PROP_FPS)) | ||
| except: | ||
| fps = None | ||
| try: | ||
| pix_fmt = decode_fourcc(cap.get(cv2.CAP_PROP_CODEC_PIXEL_FORMAT)) | ||
| except: | ||
| pix_fmt = "" | ||
| try: | ||
| frame_count = round(cap.get(cv2.CAP_PROP_FRAME_COUNT)) | ||
| except: | ||
| frame_count = None | ||
| try: | ||
| orig_im_size = ( | ||
| round(cap.get(cv2.CAP_PROP_FRAME_WIDTH)), | ||
| round(cap.get(cv2.CAP_PROP_FRAME_HEIGHT)), | ||
| ) | ||
| except: |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except block directly handles BaseException.
| except: | |
| fourcc = "" | |
| try: | |
| fps = round(cap.get(cv2.CAP_PROP_FPS)) | |
| except: | |
| fps = None | |
| try: | |
| pix_fmt = decode_fourcc(cap.get(cv2.CAP_PROP_CODEC_PIXEL_FORMAT)) | |
| except: | |
| pix_fmt = "" | |
| try: | |
| frame_count = round(cap.get(cv2.CAP_PROP_FRAME_COUNT)) | |
| except: | |
| frame_count = None | |
| try: | |
| orig_im_size = ( | |
| round(cap.get(cv2.CAP_PROP_FRAME_WIDTH)), | |
| round(cap.get(cv2.CAP_PROP_FRAME_HEIGHT)), | |
| ) | |
| except: | |
| except Exception: | |
| fourcc = "" | |
| try: | |
| fps = round(cap.get(cv2.CAP_PROP_FPS)) | |
| except Exception: | |
| fps = None | |
| try: | |
| pix_fmt = decode_fourcc(cap.get(cv2.CAP_PROP_CODEC_PIXEL_FORMAT)) | |
| except Exception: | |
| pix_fmt = "" | |
| try: | |
| frame_count = round(cap.get(cv2.CAP_PROP_FRAME_COUNT)) | |
| except Exception: | |
| frame_count = None | |
| try: | |
| orig_im_size = ( | |
| round(cap.get(cv2.CAP_PROP_FRAME_WIDTH)), | |
| round(cap.get(cv2.CAP_PROP_FRAME_HEIGHT)), | |
| ) | |
| except Exception: |
| except: | ||
| fourcc = "" | ||
| try: | ||
| fps = round(cap.get(cv2.CAP_PROP_FPS)) | ||
| except: | ||
| fps = None | ||
| try: | ||
| pix_fmt = decode_fourcc(cap.get(cv2.CAP_PROP_CODEC_PIXEL_FORMAT)) | ||
| except: | ||
| pix_fmt = "" | ||
| try: | ||
| frame_count = round(cap.get(cv2.CAP_PROP_FRAME_COUNT)) | ||
| except: | ||
| frame_count = None | ||
| try: | ||
| orig_im_size = ( | ||
| round(cap.get(cv2.CAP_PROP_FRAME_WIDTH)), | ||
| round(cap.get(cv2.CAP_PROP_FRAME_HEIGHT)), | ||
| ) | ||
| except: |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except block directly handles BaseException.
| except: | |
| fourcc = "" | |
| try: | |
| fps = round(cap.get(cv2.CAP_PROP_FPS)) | |
| except: | |
| fps = None | |
| try: | |
| pix_fmt = decode_fourcc(cap.get(cv2.CAP_PROP_CODEC_PIXEL_FORMAT)) | |
| except: | |
| pix_fmt = "" | |
| try: | |
| frame_count = round(cap.get(cv2.CAP_PROP_FRAME_COUNT)) | |
| except: | |
| frame_count = None | |
| try: | |
| orig_im_size = ( | |
| round(cap.get(cv2.CAP_PROP_FRAME_WIDTH)), | |
| round(cap.get(cv2.CAP_PROP_FRAME_HEIGHT)), | |
| ) | |
| except: | |
| except Exception: | |
| fourcc = "" | |
| try: | |
| fps = round(cap.get(cv2.CAP_PROP_FPS)) | |
| except Exception: | |
| fps = None | |
| try: | |
| pix_fmt = decode_fourcc(cap.get(cv2.CAP_PROP_CODEC_PIXEL_FORMAT)) | |
| except Exception: | |
| pix_fmt = "" | |
| try: | |
| frame_count = round(cap.get(cv2.CAP_PROP_FRAME_COUNT)) | |
| except Exception: | |
| frame_count = None | |
| try: | |
| orig_im_size = ( | |
| round(cap.get(cv2.CAP_PROP_FRAME_WIDTH)), | |
| round(cap.get(cv2.CAP_PROP_FRAME_HEIGHT)), | |
| ) | |
| except Exception: |
|
|
||
| return inf_times, im_size, meta | ||
| os.makedirs(os.path.normpath(output), exist_ok=True) | ||
| pickle.dump(data, open(out_file, "wb")) |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File is opened but is not closed.
| pickle.dump(data, open(out_file, "wb")) | |
| with open(out_file, "wb") as f: | |
| pickle.dump(data, f) |
* Update benchmark.py - changed benchmarking link; needs testing * fix DeepLabCut#112 - don't subset cpuinfo (DeepLabCut#114) * Introduce CI/CD for TensorFLow version (DeepLabCut#122) * Update .gitignore * CI/CD update python version * CI/CD update actions versions * CI/CD update trigger events * CI/CD update MacOS version * dlclibrary set version to >=0.0.6 * Poetry lock * Poetry lock * Pyproject.toml update tensorflow installation * Poetry lock * Install specific tensorflow-io-gcs-filesystem for windows * Poetry lock * Update deprecated section name * Poetry lock * CI/CD test on python 3.11, 3.12, 3.13 as well * Update testing.yml - rename to main - add tables installation --------- Co-authored-by: Mackenzie Mathis <mathis@rowland.harvard.edu> * master --> main * Update testing.yml * Update benchmark.py * Update README & benchmarking script (DeepLabCut#126) * Update README.md fix test instructions * Update check_install.py * Update check_install.py * Update check_install.py * Update pyproject.toml (DeepLabCut#125) * Update pyproject.toml - adding support for python 3.11, 3.12, 3.13 * Update testing.yml * Update testing.yml * Update pyproject.toml * Update testing.yml * Update pyproject.toml * Update poetry.lock * Update poetry.lock * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update testing.yml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update testing.yml * Update pyproject.toml * Update pyproject.toml * Update poetry.lock * Update testing.yml * Update testing.yml * Update testing.yml - remove poetry (for now) * Update testing.yml * Update testing.yml * Update testing.yml * Update testing.yml * Update testing.yml * updated poetry.lock * updated * Update testing.yml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update testing.yml testing 3.11 * Update testing.yml * Update pyproject.toml * Update pyproject.toml * Update pyproject.toml * Update testing.yml * updated test for win32 * Update testing.yml * Update testing.yml * Update testing.yml * Update testing.yml * Fix download_benchmarking_data() * Ignore __MACOSX directories * Run tests on all PRs * tensorflow-io-gcs-filesystem - python=3.9 * Pin python to 3.10 version * WIP refactor benchmarking: __init__.py * WIP refactor benchmarking: remove dupplicates * WIP refactor benchmarking: warning * WIP refactor benchmarking: close live * WIP refactor benchmarking: resize and pixels * WIP refactor benchmarking: get_sys_info * Update benchmark.py (DeepLabCut#123) * WIP refactor benchmarking: Delete dupplicate scripts * WIP refactor benchmarking: Introduce Engine * WIP refactor benchmarking: benchmark() method * WIP refactor benchmarking: benchmark_videos() * CI/CD: add torch installation * WIP refactor benchmarking: extract Engine * Refactor benchmarking: model_type in benchmark_videos() * Delete poetry lock * Fix frame_batch initialization in runner.py when low detection confidence (DeepLabCut#138) --------- Co-authored-by: Mackenzie Mathis <mathis@rowland.harvard.edu> Co-authored-by: Jonny Saunders <sneakers-the-rat@protonmail.com> Co-authored-by: Artur <35294812+arturoptophys@users.noreply.github.com>
This PR continues the work started in #121, which was left incomplete.
Specifically:
It merges the main branch, thus solving the current conflict in [WIP] Integration with DeepLabCut 3.0 - PyTorch Engine #121 with main
It merges the 3 benchmarking modules currently present in [WIP] Integration with DeepLabCut 3.0 - PyTorch Engine #121 (dlclive/benchmark.py, dlclive/benchmark_tf.py, dlclive/benchmark_pytorch.py) into a single dlclive/benchmark.py module, that works with models of both backends (TF and Torch).
Remaining work:
get_system_info()in dlclive/benchmark.py usestorchto get GPU data. We want an implementation that uses the library corresponding to the model in use (tfortorch).Then, #121 will be ready for merge into main