Skip to content

Conversation

@larryliu0820
Copy link
Contributor

@larryliu0820 larryliu0820 commented Jun 20, 2025

Currently compiling text_llm_runner.cpp requires a dependency to libsentencepiece.a and Xcode requires that library to be installed

at a directory in ${TEMP_DIR}/cmake/lib

For example on my laptop:

/Users/larryliu/Library/Developer/Xcode/DerivedData/Benchmark-dvseychddztwtucwgxpumwmyedfh/Build/Intermediates.noindex/Benchmark.build/Release-iphoneos/Tests.build/cmake/lib

Before we don't install sentencepiece, this PR changes it to install it.

To see the diff clearly: use this link https://www.internalfb.com/intern/diffing/?paste_number=1846901681

Screenshot 2025-06-20 at 2 28 19 PM

…entencepiece

Now compiling `text_llm_runner.cpp` requires a dependency to `libsentencepiece.a` and Xcode requires that library to be installed

at a directory in ${TEMP_DIR}/cmake/lib

For example on my laptop:

```
/Users/larryliu/Library/Developer/Xcode/DerivedData/Benchmark-dvseychddztwtucwgxpumwmyedfh/Build/Intermediates.noindex/Benchmark.build/Release-iphoneos/Tests.build/cmake/lib
```

Currently we don't install sentencepiece, this PR changes it to install it.
@larryliu0820 larryliu0820 requested a review from Gasoonjia as a code owner June 20, 2025 21:21
@pytorch-bot
Copy link

pytorch-bot bot commented Jun 20, 2025

🔗 Helpful Links

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

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

✅ You can merge normally! (2 Unrelated Failures)

As of commit bbbd91c with merge base 496cb05 (image):

FLAKY - The following job failed but was likely due to flakiness present on trunk:

BROKEN TRUNK - The following job failed but was 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 Jun 20, 2025
@larryliu0820 larryliu0820 added the release notes: none Do not include this in the release notes label Jun 20, 2025
@shoumikhin
Copy link
Contributor

Is this also applicable to the LLaMA demo app?

@larryliu0820
Copy link
Contributor Author

Is this also applicable to the LLaMA demo app?

Yep you are right, let me add that

Copy link
Contributor

@guangy10 guangy10 left a comment

Choose a reason for hiding this comment

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

Apple / build-benchmark-app passed. Accepted to unblock continuous benchmarking

@larryliu0820 larryliu0820 merged commit 121714a into main Jun 23, 2025
210 of 215 checks passed
@larryliu0820 larryliu0820 deleted the fix_benchmark branch June 23, 2025 07:54
hinriksnaer pushed a commit to hinriksnaer/executorch that referenced this pull request Jun 26, 2025
…entencepiece (pytorch#11834)

Currently compiling `text_llm_runner.cpp` requires a dependency to
`libsentencepiece.a` and Xcode requires that library to be installed

at a directory in ${TEMP_DIR}/cmake/lib

For example on my laptop:

```
/Users/larryliu/Library/Developer/Xcode/DerivedData/Benchmark-dvseychddztwtucwgxpumwmyedfh/Build/Intermediates.noindex/Benchmark.build/Release-iphoneos/Tests.build/cmake/lib
```

Before we don't install sentencepiece, this PR changes it to install it.

To see the diff clearly: use this link
https://www.internalfb.com/intern/diffing/?paste_number=1846901681

<img width="2446" alt="Screenshot 2025-06-20 at 2 28 19 PM"
src="https://github.com/user-attachments/assets/3d316b92-c2f9-44ae-9c77-b70f81314224"
/>
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. release notes: none Do not include this in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants