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

add onnx reverse sequence op #7771

Merged
merged 1 commit into from
May 10, 2021
Merged

add onnx reverse sequence op #7771

merged 1 commit into from
May 10, 2021

Conversation

alter-xp
Copy link
Contributor

add onnx reverse sequence op

@jroesch jroesch requested a review from mbrookhart April 13, 2021 21:21
@mbrookhart
Copy link
Contributor

@alter-xp Thanks for the contribution! Can you rebase to fix the merge conflict and fix lint? It looks great from a quick pass.

@alter-xp alter-xp force-pushed the reverse_sequence branch 2 times, most recently from a24d77b to 330bf73 Compare April 19, 2021 07:48
@mbrookhart
Copy link
Contributor

It looks like somehow to test is failing on GPU. Is that something you can debug? If you don't have access to a machine with an nvidia GPU, I can take a look.

@mbrookhart
Copy link
Contributor

@alter-xp Pinging again. This op is cropping into my view on a model, so I'd like to get this merged. I'm also interested in seeing these tests enabled as part of this PR:

tvm/tests/python/frontend/onnx/test_forward.py

Lines 4227 to 4228 in cea7cf1

"test_reversesequence_batch/",
"test_reversesequence_time/",

Is this something you can get back to?

Thanks!

@alter-xp
Copy link
Contributor Author

alter-xp commented May 8, 2021

@mbrookhart OMG, I haven't paid attention to this PR during this time. Let me see what causes the error.

@alter-xp
Copy link
Contributor Author

tvm/tests/python/frontend/onnx/test_forward.py

Lines 4227 to 4228 in cea7cf1

"test_reversesequence_batch/",
"test_reversesequence_time/",

My tests include them

mbrookhart
mbrookhart approved these changes May 10, 2021
View reviewed changes
Copy link
Contributor

@mbrookhart mbrookhart left a comment

Choose a reason for hiding this comment

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

LGTM

@mbrookhart mbrookhart merged commit 2077ee2 into apache:main May 10, 2021
@mbrookhart
Copy link
Contributor

Thanks @alter-xp

@alter-xp alter-xp deleted the reverse_sequence branch May 11, 2021 02:07
mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request May 19, 2021
@alter-xp @mehrdadh
add onnx reverse sequence op (apache#7771)
468883b 
Co-authored-by: xp224797 <[email protected]>
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Jun 17, 2021
@alter-xp
add onnx reverse sequence op (apache#7771)
5055b26 
Co-authored-by: xp224797 <[email protected]>
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Jun 17, 2021
@alter-xp @trevor-m
add onnx reverse sequence op (apache#7771)
46d0cc0 
Co-authored-by: xp224797 <[email protected]>
@junrushao junrushao mentioned this pull request Nov 1, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbrookhart mbrookhart mbrookhart approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@alter-xp @mbrookhart @jroesch