Skip to content
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

[Fix] fix bug of non-COCO dataset track demo #1504

Merged
merged 2 commits into from
Jul 27, 2022
Merged

[Fix] fix bug of non-COCO dataset track demo #1504

merged 2 commits into from
Jul 27, 2022

Conversation

daixinghome
Copy link
Contributor

@daixinghome daixinghome commented Jul 22, 2022

Motivation

Fix: #1502

Modification

BC-breaking (Optional)

Use cases (Optional)

Checklist

Before PR:

  • I have read and followed the workflow indicated in the CONTRIBUTING.md to create this PR.
  • Pre-commit or linting tools indicated in CONTRIBUTING.md are used to fix the potential lint issues.
  • Bug fixes are covered by unit tests, the case that causes the bug should be added in the unit tests.
  • New functionalities are covered by complete unit tests. If not, please add more unit tests to ensure correctness.
  • The documentation has been modified accordingly, including docstring or example tutorials.

After PR:

  • CLA has been signed and all committers have signed the CLA in this PR.

@CLAassistant
Copy link

CLAassistant commented Jul 22, 2022

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Jul 22, 2022

Codecov Report

Merging #1504 (0881466) into master (e37e758) will not change coverage.
The diff coverage is 75.00%.

❗ Current head 0881466 differs from pull request most recent head f417b06. Consider uploading reports for the commit f417b06 to get more accurate results

@@           Coverage Diff           @@
##           master    #1504   +/-   ##
=======================================
  Coverage   84.45%   84.45%           
=======================================
  Files         236      236           
  Lines       20038    20038           
  Branches     3603     3603           
=======================================
  Hits        16924    16924           
  Misses       2233     2233           
  Partials      881      881           
Flag Coverage Δ
unittests 84.36% <75.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
mmpose/apis/inference_tracking.py 61.41% <75.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e37e758...f417b06. Read the comment docs.

@Ben-Louis Ben-Louis changed the title fix bug of non-COCO dataset track demo [Fix] fix bug of non-COCO dataset track demo Jul 23, 2022
@Ben-Louis
Copy link
Collaborator

@daixinghome Thank you very much for your contribution! Could you please sign the CLA?

@ly015 ly015 requested a review from jin-s13 July 25, 2022 02:31
@daixinghome
Copy link
Contributor Author

@daixinghome daixinghome reopened this Jul 26, 2022
@daixinghome
Copy link
Contributor Author

Sorry for the mistake. Now it's ok~

@@ -36,7 +36,7 @@ def _compute_iou(bboxA, bboxB):
return iou


def _track_by_iou(res, results_last, thr):
def _track_by_iou(res, results_last, thr, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

I would recommend not using **kwargs but it seems there is no other simple solution here. @jin-s13 Do you have any suggestions?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we can use functools.partial to unify the interfaces

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can use functools.partial to unify the interfaces

Could you please give an example?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please give an example?

def _track_by_iou(res, results_last, thr): 
    # track by iou

def _track_by_oks(res, results_last, thr, sigmas): 
    # track by oks

_track = partial(_track_by_oks, sigmas=dataset_sigmas) if use_oks else _track_by_iou

if use_oks is True, then _track acts like the following function

def func(res, results_last, thr): 
    return _track_by_oks(res, results_last, thr, dataset_sigmas)

Copy link
Contributor Author

@daixinghome daixinghome Jul 27, 2022

Choose a reason for hiding this comment

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

Good idea. I've already modified the code with this method :).

@ly015 ly015 merged commit 04123e0 into open-mmlab:master Jul 27, 2022
@daixinghome daixinghome deleted the fix-track_demo-nonCOCO branch July 27, 2022 04:44
evendrow pushed a commit to evendrow/mmpose that referenced this pull request Dec 22, 2022
* fix bug of non-COCO dataset track demo

* update _track_by_oks calling with partial and revert test function

Co-authored-by: Xing Dai <xing.dai@shopee.com>
shuheilocale pushed a commit to shuheilocale/mmpose that referenced this pull request May 5, 2023
* fix bug of non-COCO dataset track demo

* update _track_by_oks calling with partial and revert test function

Co-authored-by: Xing Dai <xing.dai@shopee.com>
ajgrafton pushed a commit to ajgrafton/mmpose that referenced this pull request Mar 6, 2024
* fix bug of non-COCO dataset track demo

* update _track_by_oks calling with partial and revert test function

Co-authored-by: Xing Dai <xing.dai@shopee.com>
Sign up for free to 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.

Cannot play track demo with non-COCO format keypoints in oks_iou mode
5 participants