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

Specified models for pedestrian_tracker_demo #247

Conversation

dkustikx
Copy link
Contributor

Specified list of supported models.
Models are not supported by this demo: person-detection-retail-0002, person-reidentification-retail-0076, person-reidentification-retail-0079.

@openvino-pushbot
Copy link
Contributor

Can one of the admins verify this patch?

4 similar comments
@openvino-pushbot
Copy link
Contributor

Can one of the admins verify this patch?

@openvino-pushbot
Copy link
Contributor

Can one of the admins verify this patch?

@openvino-pushbot
Copy link
Contributor

Can one of the admins verify this patch?

@openvino-pushbot
Copy link
Contributor

Can one of the admins verify this patch?

@vladimir-dudnik
Copy link
Contributor

Jenkins please retry a build

@kchechil
Copy link
Contributor

cc @IRDonch

@IRDonch
Copy link

IRDonch commented Jul 19, 2019

Sounds like a bug in the demo to me.

@IRDonch
Copy link

IRDonch commented Jul 31, 2019

@Wovchena So would it be possible to update pedestrian_tracker_demo to make it work with the other model variants? From the descriptions, it doesn't sound like they're that different.

Copy link
Contributor

@kchechil kchechil left a comment

Choose a reason for hiding this comment

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

we need to think how to align the demo instead of narrowing down the list of supported models.

@Wovchena
Copy link
Collaborator

Wovchena commented Aug 8, 2019

  • I am suggesting to keep ReID models consistent. Currently person-reidentification-retail-0079 and person-reidentification-retail-0076 have output shape of [1, 256, 1, 1], while person-reidentification-retail-0031 - [1, 256], but person-reidentification-retail-0031.md claims it's [1, 256, 1, 1]. I think [1, 256] makes more sense. The demo expects [1, 256].
  • There is an update for detection part: pedestrian_tracker: generalize detection #286

@kchechil
Copy link
Contributor

kchechil commented Aug 8, 2019

@snosov1 @Ilya-Krylov is it possible to align all reid models to the same output format?

@snosov1
Copy link
Contributor

snosov1 commented Aug 9, 2019

@evgeny-izutov the difference in output format is, probably, an accident?

@evgeny-izutov
Copy link
Contributor

evgeny-izutov commented Aug 9, 2019

The person-reidentification-retail-0031 model uses InnerProduct layer as final layer, so the output resolution is [1, 256]. The rest models are converted to support 1x1 conv output layer (output resolution is [1, 256, 1, 1]). It looks like the demo use the old variant of IE API to determine the embedding size (256) assuming the reversed order of shape operation.
P.S. I've never seen this demo before.

@snosov1
Copy link
Contributor

snosov1 commented Aug 9, 2019

@Ilya-Krylov can we get away with the changes only on the demo side?

@Ilya-Krylov
Copy link
Contributor

@snosov1 @kchechil I think it is possible, some kind of reshape operation should be added. I would propose to do this to @LeonidBeynenson as author of the demo.

@snosov1
Copy link
Contributor

snosov1 commented Aug 9, 2019

Marvelous idea! @LeonidBeynenson what do you think?

@LeonidBeynenson
Copy link
Contributor

I created pull request #295
It contains the changes to support the reid models person-reidentification-retail-0076 and person-reidentification-retail-0079.

Please, note that the PR #295 contains models.lst file too, so if #295 is merged this PR may be closed as a duplication.

@LeonidBeynenson
Copy link
Contributor

Sorry, after small talk with @Wovchena I removed my changes from models.lst file, since now (after my PR #295) it supports all reid models and, as far as I understand, after changes by @Wovchena in #286 it supports all person detection models.

@LeonidBeynenson
Copy link
Contributor

Please, note that, if I understood @Wovchena correctly, the change in models.lst file for pedestrian tracker demo (i.e. this PR) will not be required after merging @Wovchena PR #286 and my PR #295.

@IRDonch
Copy link

IRDonch commented Aug 16, 2019

Closing, since this is obsoleted by the aforementioned PRs.

@IRDonch IRDonch closed this Aug 16, 2019
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.

None yet

10 participants