Skip to content

Conversation

@freddan80
Copy link
Collaborator

  • Create compile spec builder
  • Added default compile spec for unit tests
  • Cleaned up some redundant parameters

@pytorch-bot
Copy link

pytorch-bot bot commented Apr 11, 2024

🔗 Helpful Links

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

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

❌ 2 New Failures, 3 Unrelated Failures

As of commit 7204b6b with merge base 57dd7f1 (image):

NEW FAILURES - The following jobs have failed:

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

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 Apr 11, 2024
@freddan80 freddan80 added ciflow/trunk partner: arm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Arm labels Apr 11, 2024
@freddan80
Copy link
Collaborator Author

CI failure unrelated to this PR

* Create compile spec builder
* Added default compile spec for unit tests

Change-Id: Ieae15b213969c703fc0bfcfd99a6a84ae3412676
Signed-off-by: Fredrik Knutsson <[email protected]>
Co-authored-by: Per Åstrand <[email protected]>
@freddan80
Copy link
Collaborator Author

CI fails not related to this PR

@mergennachin mergennachin requested a review from mcr229 April 15, 2024 16:20
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

self.path_for_intermediates = None
self.permute_nhwc = False

def ethosu_compile_spec(
Copy link
Contributor

Choose a reason for hiding this comment

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

why not just do this in __init__ or you can create a @classmethod if you want to write another constructor.

self.output_format = "tosa"
return self

def dump_intermediate_tosa(self, output_path: str):
Copy link
Contributor

Choose a reason for hiding this comment

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

can all of these be @property?

So you can just do

builder.dump_intermediate_tosa = True

and

if builder.dump_intermediate_tosa

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

@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.

@facebook-github-bot
Copy link
Contributor

@digantdesai merged this pull request in 64497b7.

@mergennachin mergennachin mentioned this pull request Apr 26, 2024
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. Merged partner: arm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Arm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants