Skip to content

[TOSA] Replace Linear lowering of using Matmul with FullyConnected #616

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
Closed
wants to merge 0 commits into from
Closed

[TOSA] Replace Linear lowering of using Matmul with FullyConnected #616

wants to merge 0 commits into from

Conversation

Jerry-Ge
Copy link
Contributor

@Jerry-Ge Jerry-Ge commented Oct 4, 2023

No description provided.

@netlify
Copy link

netlify bot commented Oct 4, 2023 •
edited
Loading

✅ Deploy Preview for resplendent-gnome-14e531 ready!

Name Link
🔨 Latest commit 18dac76
🔍 Latest deploy log https://app.netlify.com/sites/resplendent-gnome-14e531/deploys/652758759cbeac0008a9e8d1
😎 Deploy Preview https://deploy-preview-616--resplendent-gnome-14e531.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 4, 2023
@cccclai
Copy link
Contributor

cccclai commented Oct 4, 2023

lgtm, let's try to add tests in case there are breakages. Looks like there are many if else

digantdesai
digantdesai requested changes Oct 5, 2023
View reviewed changes
Copy link
Contributor

@digantdesai digantdesai left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM. Left some comments, OK for now. Add TODOs as required.
But please add torch.nn.Linear tests with >2 rank.

i.e. copy simple_linear test into not_so_simple_linear with

(torch.ones(100, 20),), -> (torch.ones(5, 10, 100, 20),),

@Jerry-Ge
Copy link
Contributor Author

Jerry-Ge commented Oct 5, 2023

there're some large number of test model failures. Don't seem related? (I can't see the errors inside)

cccclai
cccclai reviewed Oct 5, 2023
View reviewed changes
Copy link
Contributor

@cccclai cccclai 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. Let's restructure and add more tests after MVP. I'm getting lost in the if-else....

@Jerry-Ge
Copy link
Contributor Author

Jerry-Ge commented Oct 6, 2023

Looks good. Let's restructure and add more tests after MVP. I'm getting lost in the if-else....

I agree. It's a pain 😓

@digantdesai digantdesai mentioned this pull request Oct 6, 2023
Closed
digantdesai
digantdesai approved these changes Oct 6, 2023
View reviewed changes
Copy link
Contributor

@digantdesai digantdesai left a comment

Choose a reason for hiding this comment

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

lgtm, let mw know when you are ready

@facebook-github-bot
Copy link
Contributor

@digantdesai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@Jerry-Ge
Copy link
Contributor Author

Jerry-Ge commented Oct 6, 2023

lgtm, let mw know when you are ready

tks, i'm ready. but not sure if this will have merge conflicts with #659 and #615

@facebook-github-bot
Copy link
Contributor

@digantdesai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@Jerry-Ge Jerry-Ge closed this Oct 12, 2023
@robell robell added the partner: arm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Arm label Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cccclai cccclai cccclai left review comments

@digantdesai digantdesai digantdesai approved these changes

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. partner: arm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Arm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Jerry-Ge @cccclai @facebook-github-bot @digantdesai @robell