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.

Sign up for GitHub

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

Jump to bottom

[Heterogeneous][Bugfix] Fix bug of wrongly generated device_map #2990

Merged
merged 6 commits into from
Apr 13, 2019
Merged

[Heterogeneous][Bugfix] Fix bug of wrongly generated device_map #2990

merged 6 commits into from
Apr 13, 2019

Conversation

imorinaga
Copy link
Contributor

For the following type of graph, device_index should be [2, 1, 2, 2] (corresponding to [a, b, copy1, c]) but generated one is [1, 1, 2, 2].

a(dev)  b(cpu)
|        |
|     copy1
 \     /
  c (dev)

The problems are

  1. While traversing graph in bottom-up propagetion, src of copy1 is wrongly propagated to a.
  2. Top-down propagation can not propagate to the nodes which have no copy in childs. (like a)

This patch fix these by checking whether each nodes is bottom-up propagatable when creating post_dfs_order_.

@zhiics could you please review?

@zhiics
Copy link
Member

zhiics commented Apr 9, 2019

@imorinaga Thanks for the fix. It looks good to me.

yzhliu
yzhliu reviewed Apr 9, 2019
View reviewed changes
Copy link
Member

@yzhliu yzhliu left a comment

Choose a reason for hiding this comment

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

otherwise good to me

tests/python/relay/test_pass_annotation.py Outdated
add mul
\ /
subtract
device_index is wrongly propagated in previous version"""
Copy link
Member

Choose a reason for hiding this comment

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

remove this comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@yzhliu yzhliu merged commit 2bf6666 into apache:master Apr 13, 2019
@yzhliu
Copy link
Member

yzhliu commented Apr 13, 2019

Thanks @imorinaga @zhiics This is merged.

@imorinaga imorinaga deleted the fix branch May 10, 2019 02:32
wweic pushed a commit to wweic/tvm that referenced this pull request May 13, 2019
@imorinaga
[Heterogeneous][Bugfix] Fix bug of wrongly generated device_map (apac…
55fa34a 
…he#2990)

* fix bug of device_index

* cpplint

* nose

* Update test_pass_annotation.py

* fix name of testcase

* delete comment
wweic pushed a commit to neo-ai/tvm that referenced this pull request May 13, 2019
@imorinaga @wweic
[Heterogeneous][Bugfix] Fix bug of wrongly generated device_map (apac…
113a325 
…he#2990)

* fix bug of device_index

* cpplint

* nose

* Update test_pass_annotation.py

* fix name of testcase

* delete comment
@yzhliu yzhliu mentioned this pull request Nov 11, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhiics zhiics zhiics approved these changes

@yzhliu yzhliu yzhliu approved these changes

Assignees
No one assigned
Labels
status: accepted
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@imorinaga @zhiics @yzhliu