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

Post-merge review of #1419 "Universal tensor transform feature" and #1481... #1476

Closed
atamazov opened this issue Mar 24, 2022 · 11 comments
Closed

Comments

@atamazov
Copy link
Contributor

atamazov commented Mar 24, 2022

UPDATE: Leftovers:

@junliume Currently this is of normal_urgency


The 1st version of this ticket (outdated)

All review comments must be resolved (the most important ones are marked with 🔴).

/cc @aska-0096 @shaojiewang @DrizztDoUrden

@junliume Currently this is of high_urgency

@junliume
Copy link
Collaborator

@aska-0096 could you follow up with Artem's comments in #1419 asap?

@aska-0096
Copy link
Collaborator

@atamazov @junliume I send the PR #1481 to cover these review suggestions, hope it works.

@DrizztDoUrden
Copy link
Contributor

@junliume
Copy link
Collaborator

junliume commented Apr 6, 2022

@aska-0096 could you take a look and respond to @DrizztDoUrden 's inquiries above? Thanks!

@aska-0096
Copy link
Collaborator

@DrizztDoUrden Sorry for late reply. I'd like to give three main advantage in tensor reorder solver.

Performance:

  1. In tensor reorder method, we re-utilize the exist batched-transpose kernel for higher performance. You can refer to performance data in PR# 1419.
  2. All Tensor reorder kernel is compiled in compile-time, and chosen via specific reorder sequence(i.e. Reorder NCHW to NHWC is (0,2,3,1)). This kind of design will decrease the run-time workload.

Completeness:
1.We support various of datatype, int8, fp16, fp32 and fp64.

@junliume
Copy link
Collaborator

junliume commented Apr 8, 2022

Can we close this one since #1481 is merged?

@atamazov
Copy link
Contributor Author

atamazov commented Apr 8, 2022

@atamazov
Copy link
Contributor Author

atamazov commented Apr 8, 2022

7 comments from #1419 (review) are not resolved yet; couple of them are red.

@atamazov atamazov changed the title Post-merge review of #1419 "Universal tensor transform feature" Post-merge review of #1419 "Universal tensor transform feature" and #1481... Apr 8, 2022
@atamazov
Copy link
Contributor Author

atamazov commented Apr 8, 2022

Description updated with #1481 (review).

@aska-0096
Copy link
Collaborator

Updated in #1515. @atamazov

@atamazov
Copy link
Contributor Author

atamazov commented Apr 10, 2022

Description updated. @junliume Now this is of normal_urgency.

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

No branches or pull requests

6 participants