Skip to content

[IE][VPU]: Enables dynamic output from middle of network support#930

Merged
ggladilov merged 3 commits intoopenvinotoolkit:masterfrom
ggladilov:vpu/gg/enables-dynamic-output-from-the-middle-of-network
Jun 16, 2020
Merged

[IE][VPU]: Enables dynamic output from middle of network support#930
ggladilov merged 3 commits intoopenvinotoolkit:masterfrom
ggladilov:vpu/gg/enables-dynamic-output-from-the-middle-of-network

Conversation

@ggladilov
Copy link
Copy Markdown
Contributor

@ggladilov ggladilov commented Jun 15, 2020 •
edited
Loading

Task

#-33465

Description

This feature is very useful for debugging dynamic networks.
Changes include modification of existing addCopyForOutputsInsideNetwork
pass to respect dynamic outputs and moving propagateDynamismToOutputs
pass after addCopyForOutputsInsideNetwork. The motivation for last change
is to avoid unnecessary copy stages due to not synchronized logic, because
previously:

  • First in Front-End (parseDSR) we mark shape data object as output
  • Then in propagateDynamismToOutputs we insert copy stage for that case.
    It's necessary if shape data object had other consumers
  • Then in convertShapeNotation we insert Gather consumer for output data object
  • Finally, addCopyForOutputsInsideNetwork inserts one more copy stage to leave
    output data object without consumers.

Signed-off-by: Gladilov, Gleb [email protected]

@ggladilov ggladilov added this to the 2021.1 milestone Jun 15, 2020
@ggladilov ggladilov requested review from a user, Maxim-Doronin and rvyunov June 15, 2020 10:12
@ggladilov ggladilov requested a review from a team as a code owner June 15, 2020 10:12
@ggladilov

This comment has been minimized.

Sign in to view
@ggladilov
Copy link
Copy Markdown
Contributor Author

ggladilov commented Jun 15, 2020

@ababushk @azhogov

I see quite strange behavior - Details link for ci/jenkins leads to job started by Hajduczenia, Jedrzej and it is without any details about inner jobs (for particular devices)

ggladilo added 2 commits June 15, 2020 16:24
@ggladilo
[IE][VPU]: Enables dynamic output from middle of network support
2a698fc 
This feature is very useful for debugging dynamic networks.
Changes include modification of existing addCopyForOutputsInsideNetwork
pass to respect dynamic outputs and moving propagateDynamismToOutputs
pass after addCopyForOutputsInsideNetwork. The motivation for last change
is to avoid unnecessary copy stages due to not synchronized logic, because
previously:

* First in Front-End (parseDSR) we mark shape data object as output
* Then in propagateDynamismToOutputs we insert copy stage for that case.
  It's necessary if shape data object had other consumers
* Then in convertShapeNotation we insert Gather consumer for output data object
* Finally, addCopyForOutputsInsideNetwork inserts one more copy stage to leave
  output data object without consumers.

Signed-off-by: Gladilov, Gleb <[email protected]>
@ggladilo
[IE][VPU]: Replaces attrs.has + attrs.get with attrs.getOrDefault
6ae7d86
@azhogov
Copy link
Copy Markdown

azhogov commented Jun 15, 2020

@ababushk @azhogov

I see quite strange behavior - Details link for ci/jenkins leads to job started by Hajduczenia, Jedrzej and it is without any details about inner jobs (for particular devices)

old job was stopped due to a new commit here.

@ggladilov
Copy link
Copy Markdown
Contributor Author

ggladilov commented Jun 15, 2020

@ababushk @azhogov
I see quite strange behavior - Details link for ci/jenkins leads to job started by Hajduczenia, Jedrzej and it is without any details about inner jobs (for particular devices)

old job was stopped due to a new commit here.

Ok, but why so strange starter name and not propagated status (it showed as failed job, not in progress)?

@ababushk
Copy link
Copy Markdown
Contributor

ababushk commented Jun 15, 2020

Ok, but why so strange starter name and not propagated status (it showed as failed job, not in progress)?

The reason it was red is the order of status update requests:

  1. New commits is pushed, new build is initiated
  2. New build updates 'ci/jenkins' check status to 'Pending' (yellow one)
  3. This new build cancels old one
  4. Old build updates 'ci/jenkins' check status to 'Failed' (red one)
  5. New build finishes and updates 'ci/jenkins' check status to 'Successful' (green one)

@ggladilov
Copy link
Copy Markdown
Contributor Author

ggladilov commented Jun 15, 2020

Ok, but why so strange starter name and not propagated status (it showed as failed job, not in progress)?

The reason it was red is the order of status update requests:

  1. New commits is pushed, new build is initiated
  2. New build updates 'ci/jenkins' check status to 'Pending' (yellow one)
  3. This new build cancels old one
  4. Old build updates 'ci/jenkins' check status to 'Failed' (red one)
  5. New build finishes and updates 'ci/jenkins' check status to 'Successful' (green one)

Why is there no status 4.1 ?
4. Old build updates 'ci/jenkins' check status to 'Failed' (red one)
4.1 New build starts and updates 'ci/jenkins' check status to 'Pending' (yellow one)
5. New build finishes and updates 'ci/jenkins' check status to 'Successful' (green one)

And how strange name can be explained?

@ggladilov ggladilov requested a review from a user June 15, 2020 17:07
@ababushk
Copy link
Copy Markdown
Contributor

ababushk commented Jun 15, 2020

Why is there no status 4.1 ?
4. Old build updates 'ci/jenkins' check status to 'Failed' (red one)
4.1 New build starts and updates 'ci/jenkins' check status to 'Pending' (yellow one)
5. New build finishes and updates 'ci/jenkins' check status to 'Successful' (green one)

Because build already started on step one. Definitely not the best behavior, so we're looking for ways to improve it.

And how strange name can be explained?
I didn't get this part, what strange name?
If you meant 'ci/jenkins' then it can be changed to anything

@ggladilov
Copy link
Copy Markdown
Contributor Author

ggladilov commented Jun 16, 2020

Why is there no status 4.1 ?
4. Old build updates 'ci/jenkins' check status to 'Failed' (red one)
4.1 New build starts and updates 'ci/jenkins' check status to 'Pending' (yellow one)
5. New build finishes and updates 'ci/jenkins' check status to 'Successful' (green one)

Because build already started on step one. Definitely not the best behavior, so we're looking for ways to improve it.

And how strange name can be explained?
I didn't get this part, what strange name?
If you meant 'ci/jenkins' then it can be changed to anything

By strange name I mean "Started by user Hajduczenia, Jedrzej"

@ggladilov
Copy link
Copy Markdown
Contributor Author

ggladilov commented Jun 16, 2020

@ababushk could you please look at validations https://github.com/openvinotoolkit/openvino/pull/930/checks?check_run_id=773432318 and https://github.com/openvinotoolkit/openvino/pull/930/checks?check_run_id=773679107?
First one is failed, but message is

##[error]We stopped hearing from agent vmssa7a4c00021T. Verify the agent machine is running and has a healthy network connection. Anything that terminates an agent process, starves it for CPU, or blocks its network access can cause this error. For more information, see: https://go.microsoft.com/fwlink/?linkid=846610

The second one hanged as far as I can see..

@ababushk
Copy link
Copy Markdown
Contributor

ababushk commented Jun 16, 2020

@ababushk could you please look at validations https://github.com/openvinotoolkit/openvino/pull/930/checks?check_run_id=773432318 and https://github.com/openvinotoolkit/openvino/pull/930/checks?check_run_id=773679107?
First one is failed, but message is

##[error]We stopped hearing from agent vmssa7a4c00021T. Verify the agent machine is running and has a healthy network connection. Anything that terminates an agent process, starves it for CPU, or blocks its network access can cause this error. For more information, see: https://go.microsoft.com/fwlink/?linkid=846610

The second one hanged as far as I can see..

This is an issue with Azure, we're looking into this, but PR still can be merged

@ggladilov ggladilov merged commit 4a85983 into openvinotoolkit:master Jun 16, 2020
@ggladilov ggladilov deleted the vpu/gg/enables-dynamic-output-from-the-middle-of-network branch June 16, 2020 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Maxim-Doronin Maxim-Doronin Maxim-Doronin approved these changes

@rvyunov rvyunov Awaiting requested review from rvyunov

+1 more reviewer
Reviewers whose approvals may not affect merge requirements

Assignees

@ggladilov ggladilov

Labels

None yet

Projects

None yet

Milestone

2021.1

Development

Successfully merging this pull request may close these issues.

5 participants

@ggladilov @azhogov @ababushk @Maxim-Doronin @ggladilo