Skip to content

Arm backend: test_pipeline improvements #8644

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

Merged
merged 1 commit into from
Feb 24, 2025

Conversation

AdrianLundell
Copy link
Collaborator

@AdrianLundell AdrianLundell commented Feb 24, 2025

  • Add OpNotSupportedPipeline for checking that ops are not delegated properly.
  • Make use_to_edge_transform_and_lower default to true since this is the recommended API.
  • Rename TestPassPipeline -> PassPipeline to avoid warnings in pytest log, and make exir_ops optional as its not used then.
  • Allow to add the first non unique stage to a pipeline w/o suffix (E.g. run_method_and_compare_outputs will rarely be used twice even though it is theoretically possible, so we don't want to refer to it as run_method_and_compare_outputs.0 if not necessary).
  • Add custom_path option to all pipelines for easily dumping artifacts.
  • Typing and documentation fixes.

cc @digantdesai @freddan80 @per @zingo @oscarandersson8218

- Add OpNotSupportedPipeline for checking that ops are not delegated
  properly.
- Make use_to_edge_transform_and_lower default to true since this is the
  recommended API.
- Rename TestPassPipeline -> PassPipeline to avoid warnings in pytest
  log, and make exir_ops optional as its not used then.
- Allow to add the first non unique stage to a pipeline w/o suffix
  (E.g. run_method_and_compare_outputs will rarely be used twice even
  though it is theoretically possible, so we don't want to refer to it
  as run_method_and_compare_outputs.0 if not necessary).
- Add custom_path option to all pipelines for easily dumping artifacts.
- Typing and documentation fixes.

Change-Id: Ic54f2691a5160259484d66f1c4c46a36f25733a4
Signed-off-by: Adrian Lundell <[email protected]>
@AdrianLundell AdrianLundell added partner: arm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Arm ciflow/trunk topic: not user facing labels Feb 24, 2025
@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 Feb 24, 2025
Copy link

pytorch-bot bot commented Feb 24, 2025

🔗 Helpful Links

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

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

❌ 2 New Failures, 1 Pending

As of commit 263de21 with merge base b6bd89d (image):

NEW FAILURES - The following jobs have failed:

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

@AdrianLundell AdrianLundell requested a review from zingo February 24, 2025 10:51
@AdrianLundell
Copy link
Collaborator Author

Unrelated errors:

Expanding inputs for image tokens in LLaVa should be done in processing. Please add `patch_size` and `vision_feature_select_strategy` to the model's processing config or set directly with `processor.patch_size = {{patch_size}}` and processor.vision_feature_select_strategy = {{vision_feature_select_strategy}}`. Using processors without these attributes in the config is deprecated and will throw an error in v4.50.
/exec: line 16: 176444 Segmentation fault      (core dumped) python -m unittest examples.models.llava.test.test_llava
Downloading shards:  50% 1/2 [08:54<08:54, 534.76s/it]
Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/ec2-user/actions-runner/_work/executorch/executorch/test-infra/.github/scripts/run_with_env_secrets.py", line 102, in <module>
  File "/opt/conda/envs/py_3.10/lib/python3.10/site-packages/urllib3/response.py", line 754, in _error_catcher
    main()
  File "/home/ec2-user/actions-runner/_work/executorch/executorch/test-infra/.github/scripts/run_with_env_secrets.py", line 98, in main
    run_cmd_or_die(f"docker exec -t {container_name} /exec")
  File "/home/ec2-user/actions-runner/_work/executorch/executorch/test-infra/.github/scripts/run_with_env_secrets.py", line 39, in run_cmd_or_die
    yield
  File "/opt/conda/envs/py_3.10/lib/python3.10/site-packages/urllib3/response.py", line 900, in _raw_read
    raise IncompleteRead(self._fp_bytes_read, self.length_remaining)
urllib3.exceptions.IncompleteRead: IncompleteRead(550088026 bytes read, 2119604526 more expected)

@zingo zingo merged commit cc5b3ed into pytorch:main Feb 24, 2025
114 of 119 checks passed
@@ -152,10 +160,11 @@ def add_stage(self, func: Callable, *args, **kwargs):

suffix = str(len(stages_containing_stage_id))

stage_id = stage_id + "." + suffix
if not suffix == "0":
Copy link
Contributor

Choose a reason for hiding this comment

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

suffix != "0". (this is not a bug, just odd; TIL that this definitely parses as not (suffix == "0"))

@AdrianLundell AdrianLundell deleted the change-995629 branch March 19, 2025 07:52
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.

4 participants