Skip to content

[IE VPU] Set name for output DSR in DTS#1011

Merged
ggladilov merged 4 commits intoopenvinotoolkit:masterfrom
Maxim-Doronin:vpu/md/set_name_to_output_DSR
Jun 30, 2020
Merged

[IE VPU] Set name for output DSR in DTS#1011
ggladilov merged 4 commits intoopenvinotoolkit:masterfrom
Maxim-Doronin:vpu/md/set_name_to_output_DSR

Conversation

@Maxim-Doronin
Copy link
Copy Markdown
Contributor

@Maxim-Doronin Maxim-Doronin commented Jun 18, 2020

Description

  • Set names for output DSR op inserted in DTS transformations
  • Enable NonZero_Transpose tests for Windows

Task

#-33722, #-31385

@Maxim-Doronin Maxim-Doronin added this to the 2021.1 milestone Jun 18, 2020
@Maxim-Doronin Maxim-Doronin self-assigned this Jun 18, 2020
@Maxim-Doronin Maxim-Doronin requested review from e-nugmanova and ggladilov and removed request for e-nugmanova and ggladilov June 19, 2020 06:36
@Maxim-Doronin Maxim-Doronin marked this pull request as ready for review June 19, 2020 06:37
@Maxim-Doronin Maxim-Doronin requested a review from a team June 19, 2020 06:37
@Maxim-Doronin Maxim-Doronin requested a review from a team as a code owner June 19, 2020 06:37
@Maxim-Doronin Maxim-Doronin requested a review from ggladilov June 19, 2020 06:37
@Maxim-Doronin Maxim-Doronin force-pushed the vpu/md/set_name_to_output_DSR branch 2 times, most recently from 86ea1e0 to 2251620 Compare June 19, 2020 11:16
Copy link
Copy Markdown
Contributor

@ggladilov ggladilov left a comment

Choose a reason for hiding this comment

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

LGTM in general, but may be it worthies to check API usage with @GlebKazantaev

Comment on lines 48 to 50
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.

Nitpick: use move semantic here:

    auto outDSR = std::make_shared<ngraph::vpu::op::DynamicShapeResolver>(copied, shape);
    outDSR->set_friendly_name(eltwise->get_friendly_name());
    ngraph::replace_node(std::move(eltwise), std::move(outDSR));

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.

If you agree, probably it is applicable for other places as well

@Maxim-Doronin Maxim-Doronin force-pushed the vpu/md/set_name_to_output_DSR branch from ebc29c3 to 4abff22 Compare June 19, 2020 16:06
@Maxim-Doronin Maxim-Doronin force-pushed the vpu/md/set_name_to_output_DSR branch from 4abff22 to 2851a94 Compare June 29, 2020 09:01
@Maxim-Doronin Maxim-Doronin requested a review from rvyunov June 29, 2020 11:12
@Maxim-Doronin Maxim-Doronin force-pushed the vpu/md/set_name_to_output_DSR branch from 2851a94 to 327a91d Compare June 29, 2020 11:40
@Maxim-Doronin
Copy link
Copy Markdown
Contributor Author

@rvyunov, could you review changes, please?

@rvyunov
Copy link
Copy Markdown
Contributor

rvyunov commented Jun 29, 2020

It looks good to me 👍 But internal windows validation seems broken. What about it?

@Maxim-Doronin
Copy link
Copy Markdown
Contributor Author

Maxim-Doronin commented Jun 29, 2020

It looks good to me 👍 But internal windows validation seems broken. What about it?

It is not a related issue. Nightly validation from master #120 has the same failures.
You can look at Win10 + MyriadX configuration at pre-commit. All related tests passed.

Please, mark this PR as approved if you have no objections.

@ggladilov ggladilov merged commit 3790b35 into openvinotoolkit:master Jun 30, 2020
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.

3 participants