Skip to content

[WIP] reformat trackers#18627

Closed
ZhiyuanChen wants to merge 6 commits intoopencv:4.xfrom
ZhiyuanChen:tracker
Closed

[WIP] reformat trackers#18627
ZhiyuanChen wants to merge 6 commits intoopencv:4.xfrom
ZhiyuanChen:tracker

Conversation

@ZhiyuanChen
Copy link
Copy Markdown
Contributor

@ZhiyuanChen ZhiyuanChen commented Oct 19, 2020

force_builders_only=Linux x64,docs

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@ZhiyuanChen ZhiyuanChen mentioned this pull request Oct 19, 2020
6 tasks
@dkurt
Copy link
Copy Markdown
Member

dkurt commented Oct 20, 2020

@ieliz, can you please review?

@ieliz
Copy link
Copy Markdown
Contributor

ieliz commented Oct 20, 2020

LGTM!
Waiting for the next changes.

@ZhiyuanChen ZhiyuanChen marked this pull request as draft October 22, 2020 12:49
@ieliz
Copy link
Copy Markdown
Contributor

ieliz commented Oct 22, 2020

Also, I think @l-bat, @bhack and @jinyup100 could look at these changes :)

Comment on lines 11 to 14

import numpy as np
import cv2 as cv
import argparse
import cv2 as cv
import numpy as np
import sys
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.

According to PEP 8.

Imports should be grouped in the following order:

  1. Standard library imports.
  2. Related third party imports.
  3. Local application/library specific imports.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

imports will be handle by auto format when it is done

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please import numpy before cv2 (otherwise it may lead to import error)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Resolved

elif self.windowing == "uniform":
self.window = np.ones((self.score_size, self.score_size))
self.window = np.tile(self.window.flatten(), self.anchor_num)
self.score = []
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.

I think this change unnecessary

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

score will be removed in later commits

self.ratios = [0.33, 0.5, 1, 2, 3]
self.scales = [8, ]
self.anchor_num = len(self.ratios) * len(self.scales)
self.score_size = (self.instance_size - self.exemplar_size) // self.anchor_stride + 1
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.

I think this change unnecessary

Copy link
Copy Markdown
Contributor Author

@ZhiyuanChen ZhiyuanChen Oct 24, 2020

Choose a reason for hiding this comment

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

Is it normal for two similar classes to have different names for the same attributes?

Comment on lines -25 to -27
self.ratios = [0.33, 0.5, 1, 2, 3]
self.scales = [8, ]
self.anchor_num = len(self.ratios) * len(self.scales)
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.

Why these variables should be renamed?

Copy link
Copy Markdown
Contributor Author

@ZhiyuanChen ZhiyuanChen Oct 24, 2020

Choose a reason for hiding this comment

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

Is it normal for two similar classes to have different names for the same attributes?

Comment on lines -84 to -86
self.track_context_amount = 0.5
self.track_exemplar_size = 127
self.track_instance_size = 255
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.

Why these variables should be renamed?

Copy link
Copy Markdown
Contributor Author

@ZhiyuanChen ZhiyuanChen Oct 24, 2020

Choose a reason for hiding this comment

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

Is it normal for two similar classes to have different names for the same attributes?

@jinyup100
Copy link
Copy Markdown
Contributor

@l-bat @b-hack @ieliz @ZhiyuanChen This PR seems to suggest making a new directory "trackers" within opencv/samples/dnn . I was wondering where the OpenCV team stands regarding making additional directories in the opencv/samples/dnn? Or is it more of a case of updating the tracker module in the opencv_contrib?

@asmorkalov
Copy link
Copy Markdown
Contributor

@ZhiyuanChen @l-bat Do you have progress on the patch?

@ZhiyuanChen
Copy link
Copy Markdown
Contributor Author

@ZhiyuanChen @l-bat Do you have progress on the patch?

Yes, there are. Hopefully I could push something new next Wednesday.

xfs_1, xfs_2, xfs_3 = self.search_net.forward(outNames)
self.rpn_head.setInput(np.stack([self.zfs_1, self.zfs_2, self.zfs_3]), 'input_1')
self.rpn_head.setInput(np.stack([self.zfs_1, self.zfs_2, self.zfs_3]),
'input_1')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please avoid such line break changes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed, this is probably resulted by 80 charter length limit following PEP-8

type=str,
help=
'Path to input video file. Skip this argument to capture frames from a camera.'
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems very synthetic. Please tune IDE/code linter to ignore long lines

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you

@asmorkalov
Copy link
Copy Markdown
Contributor

@ZhiyuanChen @dkurt Any activity here?

@ZhiyuanChen
Copy link
Copy Markdown
Contributor Author

@ZhiyuanChen @dkurt Any activity here?

I am really sorry, I'll start working on this next week.

@alalek
Copy link
Copy Markdown
Member

alalek commented Aug 28, 2021

DaSiamRPN logic have been moved into OpenCV modules as C++ code (see #20036).
So there is no more dasiamrpn.py Python-only sample.

If you want to update logic with extra parameters then please update C++ code instead.

@alalek
Copy link
Copy Markdown
Member

alalek commented Jan 16, 2022

Lets close this for now. Feel free to re-open if there are updates.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants