Skip to content

Arm backend: Tosa tools update #9451

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

Closed
wants to merge 3 commits into from
Closed

Conversation

per
Copy link
Collaborator

@per per commented Mar 20, 2025

Summary

Move the tosa tools v0.80 to a separate namespace to be able to install the upcoming 1.0 release
in it's place.

Test plan

Tested through the Arm backend unit test flow.

cc @digantdesai @freddan80 @zingo @oscarandersson8218

@per per added partner: arm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Arm ciflow/trunk topic: not user facing labels Mar 20, 2025
@per per requested a review from digantdesai as a code owner March 20, 2025 14:58
Copy link

pytorch-bot bot commented Mar 20, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/9451

Note: Links to docs will display an error until the docs builds have been completed.

❌ 2 New Failures

As of commit f793f79 with merge base 9b7a878 (image):

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@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 Mar 20, 2025
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.

I think I understand what is happening but can't say I understand the rationale? Are we planning to have two versions co-exist?

Also this will cause chaos internally if we merge it now :)

@per per force-pushed the tosa_tools_update_1 branch from 4367932 to 0fe745b Compare March 21, 2025 07:45
@per
Copy link
Collaborator Author

per commented Mar 21, 2025

@digantdesai Yes, we plan to have the two versions coexist.
The rationale is that we want to start test the 1.0 TOSA flow (including github CI) and also be able to test backend compilers transition to 1.0 (on internal CI).
When everything is green on 1.0 we'll pull out the 0.80 version.

@digantdesai
Copy link
Contributor

When everything is green on 1.0 we'll pull out the 0.80 version.

Though OK for GH OSS, it will be a headache internally. Curious, can we test the 1.0 on a branch on GH and once we are ready we can just drop 0.80? Assuming 1.0 is BC with 0.80 of tosa tools. This will save some throwaway efforts on my side internally.

@per per force-pushed the tosa_tools_update_1 branch from 0fe745b to 128cd53 Compare March 24, 2025 11:24
@per
Copy link
Collaborator Author

per commented Mar 25, 2025

1.0 on a branch on GH and once we are ready we can just drop 0.80? Assuming 1.0 is BC with 0.80 of tosa tools

The 1.0 tooling is not backward compatible with 0.80 unfortunately. Getting this in will let us develop the 1.0 support for reference in parallel as the compiler gets it's support.

@per per force-pushed the tosa_tools_update_1 branch 2 times, most recently from 34eb08b to 6434576 Compare March 26, 2025 08:22
@per per requested review from zingo and oscarandersson8218 March 26, 2025 14:41
@per per force-pushed the tosa_tools_update_1 branch 2 times, most recently from a524a54 to 18a2bbb Compare March 26, 2025 20:30
@per
Copy link
Collaborator Author

per commented Mar 26, 2025

@digantdesai Good to go?

@per
Copy link
Collaborator Author

per commented Mar 26, 2025

@per per force-pushed the tosa_tools_update_1 branch 6 times, most recently from 7064d03 to 2454103 Compare March 28, 2025 14:31
@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.

@@ -0,0 +1,154 @@
From 20c2059723d5c6952cecfb7fcde92601639ef825 Mon Sep 17 00:00:00 2001
Copy link
Contributor

Choose a reason for hiding this comment

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

we need to pull this in

@@ -0,0 +1,283 @@
From b3c8c3f779a7e051826f317598fb831fa9cfe923 Mon Sep 17 00:00:00 2001
Copy link
Contributor

Choose a reason for hiding this comment

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

And this

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.

I pulled this in, let's see how many different ways it fails, and what I need to do to fix it. Let's hold on to merging this until then? I should be able to do this early next week.

@digantdesai
Copy link
Contributor

Sigh something didn't work right, running the internal CI again

@zingo
Copy link
Collaborator

zingo commented Apr 4, 2025

@digantdesai

Sigh something didn't work right, running the internal CI again

Is the problems due to the change or other random things?
Is there some error you can share?

@per per force-pushed the tosa_tools_update_1 branch from df483db to 54c04ad Compare April 4, 2025 10:05
@digantdesai
Copy link
Contributor

ok so the pull worked this time, as expected broke a bunch of things internally because this module doesn't exist. I am on PTO today, I will take a stab at this on Mon. Sorry for blocking this :(

@per per force-pushed the tosa_tools_update_1 branch from 54c04ad to de20246 Compare April 7, 2025 14:57
@digantdesai
Copy link
Contributor

Ok got this to work internally. I can merge it with internal stack or you can merge it and I can forward fix it.

If you are doing it, please rebase, fix where and erf ops, and I can pull in again, while it merges with internal, and get ready to apply the fix.

Else if you want me to merge it then perhaps, I have to commandeer this one and merge it with few other commits.

@per per force-pushed the tosa_tools_update_1 branch 2 times, most recently from 305176c to 3d560f9 Compare April 8, 2025 13:45
@per
Copy link
Collaborator Author

per commented Apr 8, 2025

Thank you Digant! I've rebased and fixed the linterror in erf, couldn't see anything in where though. Let me know when you've imported it and I can merge.

per added 2 commits April 8, 2025 18:43
In preparation to move to TOSA 1.0 release the 0.80 package is moved
to it's own namespace to be able to handle both versions during the
transition period.

Signed-off-by: Per Åstrand <[email protected]>
Change-Id: Ib653ae81125052f5105ff124b59155018c98aef5
NFC that moves to use the TOSA tools from a different namespace.
Preparation to run 0.80 and 1.0 along side during the transition
period.

Signed-off-by: Per Åstrand <[email protected]>
Change-Id: I69d6d6a46fa95213afde8952f151fc9b41065099
@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.

@per
Copy link
Collaborator Author

per commented Apr 9, 2025

Failures are caused by #9938.

@SS-JIA
Copy link
Contributor

SS-JIA commented Apr 9, 2025

Will revert the PR as it is causing some internal failures as well.

@SS-JIA
Copy link
Contributor

SS-JIA commented Apr 10, 2025

Decided to try and forward fix with #10043 since the PR has been synced to our internal repo already. PTAL!

digantdesai pushed a commit to digantdesai/executorch-1 that referenced this pull request Apr 10, 2025
Summary:
Move the tosa tools v0.80 to a separate namespace to be able to install the upcoming 1.0 release
in it's place.

Pull Request resolved: pytorch#9451

Test Plan:
Tested through the Arm backend unit test flow.

cc digantdesai freddan80 zingo oscarandersson8218

Reviewed By: limintang

Differential Revision: D72071739

Pulled By: digantdesai
digantdesai pushed a commit to digantdesai/executorch-1 that referenced this pull request Apr 10, 2025
Summary:
Pull Request resolved: pytorch#10075

Move the tosa tools v0.80 to a separate namespace to be able to install the upcoming 1.0 release
in it's place.

Pull Request resolved: pytorch#9451

Test Plan:
Tested through the Arm backend unit test flow.

cc digantdesai freddan80 zingo oscarandersson8218

Reviewed By: SS-JIA, limintang

Differential Revision: D72071739

Pulled By: digantdesai
digantdesai pushed a commit to digantdesai/executorch-1 that referenced this pull request Apr 10, 2025
Summary:
Pull Request resolved: pytorch#10075

Move the tosa tools v0.80 to a separate namespace to be able to install the upcoming 1.0 release
in it's place.

Pull Request resolved: pytorch#9451

Test Plan:
Tested through the Arm backend unit test flow.

cc digantdesai freddan80 zingo oscarandersson8218

Reviewed By: SS-JIA, limintang

Differential Revision: D72071739

Pulled By: digantdesai
facebook-github-bot pushed a commit that referenced this pull request Apr 10, 2025
Differential Revision: D72071739

Pull Request resolved: #10075
kirklandsign pushed a commit that referenced this pull request Apr 11, 2025
Differential Revision: D72071739

Pull Request resolved: #10075
@per per closed this Apr 15, 2025
@per per deleted the tosa_tools_update_1 branch April 15, 2025 06:58
keyprocedure pushed a commit to keyprocedure/executorch that referenced this pull request Apr 21, 2025
Differential Revision: D72071739

Pull Request resolved: pytorch#10075
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk 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 topic: not user facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants