Skip to content
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

[retry] Use pytorch-labs/tokenizers and remove tokenizer/ (#1401) #1443

Merged
merged 4 commits into from
Jan 3, 2025

Conversation

larryliu0820
Copy link
Contributor

Summary: Retry of #1401

Test Plan: Re-run the repro command in #1413:

python3 torchchat.py generate llama3.2-1b-base --prompt "write me a story about a boy and his bear"

Reviewers:

Subscribers:

Tasks:

Tags:

Summary: Retry of #1401

Test Plan: Re-run the repro command in #1413:

```
python3 torchchat.py generate llama3.2-1b-base --prompt "write me a story about a boy and his bear"

```

Reviewers:

Subscribers:

Tasks:

Tags:
Copy link

pytorch-bot bot commented Dec 26, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/torchchat/1443

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

⏳ No Failures, 1 Pending

As of commit 2075fd7 with merge base 019f76f (image):
💚 Looks good so far! There are no failures yet. 💚

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 Meta Open Source bot. label Dec 26, 2024
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@@ -93,7 +93,7 @@ popd
if [[ "$TARGET" == "et" ]]; then
cmake -S . -B ./cmake-out -DCMAKE_PREFIX_PATH=`python3 -c 'import torch;print(torch.utils.cmake_prefix_path)'` -DLINK_TORCHAO_OPS="${LINK_TORCHAO_OPS}" -DET_USE_ADAPTIVE_THREADS=ON -DCMAKE_CXX_FLAGS="-D_GLIBCXX_USE_CXX11_ABI=1" -G Ninja
else
cmake -S . -B ./cmake-out -DCMAKE_PREFIX_PATH=`python3 -c 'import torch;print(torch.utils.cmake_prefix_path)'` -DLINK_TORCHAO_OPS="${LINK_TORCHAO_OPS}" -DCMAKE_CXX_FLAGS="-D_GLIBCXX_USE_CXX11_ABI=0" -G Ninja
cmake -S . -B ./cmake-out -DCMAKE_PREFIX_PATH=`python3 -c 'import torch;print(torch.utils.cmake_prefix_path)'` -DLINK_TORCHAO_OPS="${LINK_TORCHAO_OPS}" -DCMAKE_CXX_FLAGS="-D_GLIBCXX_USE_CXX11_ABI=1" -G Ninja
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Getting "model file path should not be empty." from sentencepiece processor.

According to chatgpt:

The _GLIBCXX_USE_CXX11_ABI macro is related to the Application Binary Interface (ABI) used by the GNU C++ Standard Library.
In 2011, the C++11 standard introduced a new ABI for C++ objects, which changed the way that standard library objects, including std::string, are laid out in memory. This new ABI is enabled by default in GCC 5 and later versions.
However, when _GLIBCXX_USE_CXX11_ABI=0 is defined, it tells the compiler to use the older, pre-C++11 ABI instead. This can cause issues with std::string objects because the older ABI uses a different memory layout for these objects.
When you pass a const char* pointer to a function that expects a std::string object, the compiler needs to create a temporary std::string object from the pointer. However, if the ABI is set to the older version, the compiler may not be able to correctly create this temporary object, resulting in an empty string being passed to the function.
This issue is likely due to the fact that the older ABI uses a different memory layout for std::string objects, which includes a pointer to the string data, a length field, and a capacity field. When the compiler creates a temporary std::string object from a const char* pointer, it needs to initialize these fields correctly. However, if the ABI is set to the older version, the compiler may not be able to correctly initialize these fields, resulting in an empty string.

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@Jack-Khuu
Copy link
Contributor

Thanks for the fix!!

xrf: #1440

@Jack-Khuu Jack-Khuu merged commit 83e7624 into main Jan 3, 2025
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants