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

Fix unused-variables warnings about EmbeddingBag ops #1220

Merged
merged 1 commit into from
Aug 15, 2022

Conversation

ramiro050
Copy link
Collaborator

According to the documentation for
torch.embedding_bag (https://pytorch.org/docs/stable/generated/torch.nn.functional.embedding_bag.html),
the default value for scale_grad_by_freq is False.

Copy link
Collaborator

@vidsinghal vidsinghal left a comment

Choose a reason for hiding this comment

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

looks good to me!
Can you look into why the E2E test for embeddingBag is failing?

According to the documentation for
`torch.embedding_bag` (https://pytorch.org/docs/stable/generated/torch.nn.functional.embedding_bag.html),
the default value for `scale_grad_by_freq` is False.
@ramiro050
Copy link
Collaborator Author

looks good to me! Can you look into why the E2E test for embeddingBag is failing?

I was checking for if (!scaleGradByFreqBool) here

@ramiro050 ramiro050 merged commit 9d6ee48 into llvm:main Aug 15, 2022
@ramiro050 ramiro050 deleted the fix-warnings-embedding branch August 15, 2022 16:44
qedawkins pushed a commit to nod-ai/torch-mlir that referenced this pull request Oct 3, 2022
* draft

Signed-off-by: chentong319 <[email protected]>

* move type converter

Signed-off-by: chentong319 <[email protected]>

* lowering

Signed-off-by: chentong319 <[email protected]>

* new Cast interface

Signed-off-by: chentong319 <[email protected]>

* more verify and shape

Signed-off-by: chentong319 <[email protected]>

* add warning

Signed-off-by: chentong319 <[email protected]>

* allow Seq type

Signed-off-by: chentong319 <[email protected]>

* pass runtime test

Signed-off-by: chentong319 <[email protected]>

* rm SequenceConstruct

Signed-off-by: chentong319 <[email protected]>

* rm construct

Signed-off-by: chentong319 <[email protected]>

* lit test

Signed-off-by: chentong319 <[email protected]>

* format

Signed-off-by: chentong319 <[email protected]>

* fix merge

Signed-off-by: chentong319 <[email protected]>

* comment

Signed-off-by: chentong319 <[email protected]>

* review respond

Signed-off-by: chentong319 <[email protected]>

* fix

Signed-off-by: chentong319 <[email protected]>

* format

Signed-off-by: chentong319 <[email protected]>

Co-authored-by: Tung D. Le <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants