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

Use 0D tensors for scalar inputs in testcases #3583

Closed
wants to merge 1 commit into from

Conversation

shinh
Copy link
Member

@shinh shinh commented Jul 15, 2021

Description

Testcases of OneHot, SequenceInsert, and NonMaxSuppression were using 1D tensor as scalar inputs but their documents say they accept scalars.

Motivation and Context

These tests look inconsistent with the document and I could not find any document which mentions 1D tensor with a single element can be treated as a scalar. Especially, the document of SequenceInsert clearly states position should be "tensor of empty shape".

I felt the second input of TopK op should be also a scalar, but the document says it takes 1D tensor with a single element.

@shinh shinh requested a review from a team as a code owner July 15, 2021 08:29
@shinh shinh force-pushed the 1d-to-scalar branch 2 times, most recently from 9ac0d38 to 0b967e1 Compare July 15, 2021 08:41
Signed-off-by: Shinichiro Hamaji <shinichiro.hamaji@gmail.com>
@jcwchen jcwchen added the run release CIs Use this label to trigger release tests in CI label Jul 15, 2021
@askhade
Copy link
Contributor

askhade commented Sep 21, 2021

@shinh : I agree with you regarding this inconsistency. I have also encountered this issue more than once and I believe we need to clarify the treatment of scalars and 1D tensors of len 1. Should they be treated as same or not.

here is the issue I created: #2428

I will propose this topic for Oct Arch-Infra-Op Sig meeting.

@askhade askhade added this to the 1.11 milestone Sep 21, 2021
Copy link
Contributor

@askhade askhade left a comment

Choose a reason for hiding this comment

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

PR looks good to me. Please resolve conflicts and then we can merge this

@jcwchen jcwchen removed this from the 1.11 milestone Jun 9, 2022
@github-actions github-actions bot closed this Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-pr-activity run release CIs Use this label to trigger release tests in CI
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants