Fix deeplabcut.analyze_images() with CTD model - #2990
Conversation
MMathisLab
left a comment
MMathisLab
left a comment
There was a problem hiding this comment.
lgtm, but I did not test code
| max_individuals=max_individuals, | ||
| device=device, | ||
| progress_bar=progress_bar, | ||
| # TODO: when COND_TOP_DOWN SuperAnimal models will be released - create & pass a conditions provider |
There was a problem hiding this comment.
maybe already add this; we can release the mouse model now cc @AlexEMG @n-poulsen ?
|
@n-poulsen I'd have a quick question for you: This Pull Request addresses the issue of To fix this, I create and pass a However, I noticed in the Do you rather think that:
or
Thanks in advance for sharing your opinion on this 😃🙏 |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a crash in deeplabcut.analyze_images() when using CTD models by introducing a conditions provider and updating downstream calls and visualization to handle CTD-specific predictions.
- Added a
ctd_conditionsargument andcond_providerlogic inanalyze_images/analyze_image_folder - Updated
create_labeled_imagesto only draw boxes when bothbboxesandbbox_scoresare present - Extended the high-level
analyze_imagesincompat.pyto forward CTD settings via**torch_kwargs
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| deeplabcut/pose_estimation_pytorch/apis/visualization.py | Only plot bounding boxes when both bboxes and bbox_scores exist |
| deeplabcut/pose_estimation_pytorch/apis/analyze_images.py | Support CTD models by adding ctd_conditions, instantiating/passing cond_provider, and updating function signatures |
| deeplabcut/compat.py | Accept and forward extra PyTorch args (ctd_conditions) through **torch_kwargs |
Comments suppressed due to low confidence (2)
deeplabcut/pose_estimation_pytorch/apis/analyze_images.py:237
- The new CTD support path for
analyze_images()is not covered by existing integration tests. Consider adding tests that calldeeplabcut.analyze_images()with a CTD model to ensure this functionality is verified.
ctd_conditions: dict | CondFromModel | None = None,
deeplabcut/pose_estimation_pytorch/apis/analyze_images.py:522
- The docstring for
plot_images_cocoshould be updated to include the newcond_providerparameter in its Args section, explaining its purpose and expected type.
cond_provider: CondFromModel | None = None,
| max_individuals=max_individuals, | ||
| device=device, | ||
| progress_bar=progress_bar, | ||
| # TODO: when COND_TOP_DOWN SuperAnimal models will be released - create & pass a conditions provider |
There was a problem hiding this comment.
[nitpick] Consider either resolving this TODO by implementing the condition provider logic for COND_TOP_DOWN SuperAnimal models or referencing an issue for future tracking to avoid stale comments.
| # TODO: when COND_TOP_DOWN SuperAnimal models will be released - create & pass a conditions provider | |
| # TODO: when COND_TOP_DOWN SuperAnimal models will be released - create & pass a conditions provider | |
| # Tracking issue: https://github.com/DeepLabCut/DeepLabCut/issues/<issue_number> |
|
@maximpavliv see review above, thanks! |
|
Status on thus PR? |
|
@MMathisLab Ready for review and merge! |
The integration of CTD models adapted the
deeplabcut.analyze_videos()to support CTD models, but missed adapting thedeeplabcut.analyze_images()API method accordingly, which resulted in a crash when trying to usedeeplabcut.analyze_images()with a CTD model.This Pull Request fixes this by:
Creating and using a conditions provider (
cond_provider) when creating a pose inference runner inanalyze_image_folder().Adding a
**torch_kwargsargument to theanalyze_images()in compat.py (to allow passingctd_conditionsat this level). If left unspecified - the conditions are loaded accordingly to the specifications in the pytorch_config.yaml file.The
create_labeled_images()method in deeplabcut/pose_estimation_pytorch/apis/visualization.py has been adapted as well. In fact, previously, the method assumed that, if theimage_predictionscontained a"bboxes"key - it also contained a"bbox_scores"key, both being generated by detectors in Top-Down architectures. Now, with the addition of CTD models, theimage_predictionscontain a"bboxes"key, but the bounding boxes have been built using the condition poses, thereforeimage_predictionsdoesn't contain a"bbox_scores"key. In summary, with this change, the method doesn't try to draw bounding boxes when using a CTD model./!\ Spotted limitation: the
deeplabcut.analyze_images()API method is never tested in the integration tests!