Optionally ignore utf-8 decoding error when converting std::string to python str. #97282

shuminghu · 2023-03-21T20:36:47Z

Summary: When language models use c++ tokenizer, outputs are a c++ strings that are not necessarily valid utf-8 encodings. Default pybind11 casting uses strict utf-8 decoding. We relax the decoding using 'ignore' argument.

Test Plan: https://www.internalfb.com/intern/testinfra/testrun/6473924609918070

Reviewed By: Nayef211

Differential Revision: D43970697

pytorch-bot · 2023-03-21T20:36:50Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

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

⏳ No Failures, 5 Pending

As of commit 237daaa:
💚 Looks good so far! There are no failures yet. 💚

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

linux-foundation-easycla · 2023-03-21T20:36:51Z

The committers listed above are authorized under a signed CLA.

✅ login: shuminghu / name: Shuming Hu (dd7a4c0)

facebook-github-bot · 2023-03-21T20:37:30Z

This pull request was exported from Phabricator. Differential Revision: D43970697

… python str. (#97282) Summary: X-link: pytorch/pytorch#97282 Pull Request resolved: pytorch#2126 When language models use c++ tokenizer, outputs are a c++ strings that are not necessarily valid utf-8 encodings. Default pybind11 casting uses strict utf-8 decoding. We relax the decoding using 'ignore' argument. Reviewed By: Nayef211 Differential Revision: D43970697 fbshipit-source-id: 37da8270cfd4ae11a43aeb7ab7093edd7d800cee

facebook-github-bot · 2023-03-21T22:32:55Z

This pull request was exported from Phabricator. Differential Revision: D43970697

… python str. (pytorch#97282) Summary: Pull Request resolved: pytorch#97282 X-link: pytorch/text#2126 When language models use c++ tokenizer, outputs are a c++ strings that are not necessarily valid utf-8 encodings. Default pybind11 casting uses strict utf-8 decoding. We relax the decoding using 'ignore' argument. Test Plan: https://www.internalfb.com/intern/testinfra/testrun/6473924609918070 Reviewed By: Nayef211 Differential Revision: D43970697 fbshipit-source-id: 3202461664252f309ff9a63b35faaf642e92a81a

facebook-github-bot · 2023-03-21T22:39:25Z

This pull request was exported from Phabricator. Differential Revision: D43970697

… python str. (pytorch#97282) Summary: Pull Request resolved: pytorch#97282 X-link: pytorch/text#2126 When language models use c++ tokenizer, outputs are a c++ strings that are not necessarily valid utf-8 encodings. Default pybind11 casting uses strict utf-8 decoding. We relax the decoding using 'ignore' argument. Test Plan: https://www.internalfb.com/intern/testinfra/testrun/6473924609918070 Reviewed By: Nayef211 Differential Revision: D43970697 fbshipit-source-id: 88954cce2894909bab6a9f7f26d8a41d9652d9fc

davidberard98

LGTM!

a few nits (inline)
can you make sure the lint is passing? one of the links shows it's failing but I can't quite find it. (in the pytorch repo, lint failures will get reverted)

davidberard98 · 2023-03-22T00:34:02Z

torch/script.h

@@ -5,6 +5,7 @@
 #include <torch/csrc/autograd/custom_function.h>
 #include <torch/csrc/autograd/generated/variable_factories.h>
 #include <torch/csrc/autograd/grad_mode.h>
+#include <torch/csrc/jit/python/utf8_decoding_ignore.h>


nit: is this necessary?

This is needed to for registering bindings in the follow-up change in torchtext/csrc/register_torchbindings.cpp, since it only includes torch/script.h.

hmm can we just include torch/csrc/jit/python/utf8_decoding_ignore.h in register_torchbindings.cpp?

not a big deal but since this is sort of a private api, we probably don't need to include it in torch/script.h (as you can see the current script.h includes a pretty minimal list of .h files)

Wierd. It fails with

fbcode/pytorch/text/torchtext/csrc/register_torchbindings.cpp:215:20: error: no member named 'setUTF8DecodingIgnore' in namespace 'torch::jit' &torch::jit::setUTF8DecodingIgnore);

when utf8_decoding_ignore.h is moved from torch/script.h to torchtext/csrc/register_torchbindings.cpp.

hmm and this is after adding the .h include to both torchtext/csrc/register_pybindings.cpp and torchtext/csrc/register_torchingbindings.cpp

Worked. I accidentally added it to files under /xplat 😅

shuminghu · 2023-03-22T00:43:31Z

The lint fix has already been applied. It's from the formatting of adjacent but un-modified code.

… python str. (#97282) Summary: X-link: pytorch/pytorch#97282 Pull Request resolved: pytorch#2126 When language models use c++ tokenizer, outputs are a c++ strings that are not necessarily valid utf-8 encodings. Default pybind11 casting uses strict utf-8 decoding. We relax the decoding using 'ignore' argument. Reviewed By: Nayef211 Differential Revision: D43970697 fbshipit-source-id: 4988147e6905d1fb8096bf6d172ab8d8952b49b0

facebook-github-bot · 2023-03-22T01:40:18Z

This pull request was exported from Phabricator. Differential Revision: D43970697

… python str. (pytorch#97282) Summary: Pull Request resolved: pytorch#97282 X-link: pytorch/text#2126 When language models use c++ tokenizer, outputs are a c++ strings that are not necessarily valid utf-8 encodings. Default pybind11 casting uses strict utf-8 decoding. We relax the decoding using 'ignore' argument. Test Plan: https://www.internalfb.com/intern/testinfra/testrun/4503599786612705 Reviewed By: Nayef211 Differential Revision: D43970697 fbshipit-source-id: 262b3e9165e50d893a72f162705956102f1143bc

… python str. (#97282) Summary: X-link: pytorch/pytorch#97282 Pull Request resolved: pytorch#2126 When language models use c++ tokenizer, outputs are a c++ strings that are not necessarily valid utf-8 encodings. Default pybind11 casting uses strict utf-8 decoding. We relax the decoding using 'ignore' argument. Reviewed By: Nayef211 Differential Revision: D43970697 fbshipit-source-id: 89cee96315440bb54f5d5e70665ac2fc4ee75e1b

facebook-github-bot · 2023-03-22T01:48:43Z

This pull request was exported from Phabricator. Differential Revision: D43970697

… python str. (pytorch#97282) Summary: Pull Request resolved: pytorch#97282 X-link: pytorch/text#2126 When language models use c++ tokenizer, outputs are a c++ strings that are not necessarily valid utf-8 encodings. Default pybind11 casting uses strict utf-8 decoding. We relax the decoding using 'ignore' argument. Test Plan: https://www.internalfb.com/intern/testinfra/testrun/4503599786612705 Reviewed By: Nayef211 Differential Revision: D43970697 fbshipit-source-id: a871d2537a6c3aa26f1858be2484320f92184e37

… python str. (pytorch#97282) Summary: Pull Request resolved: pytorch#97282 X-link: pytorch/text#2126 When language models use c++ tokenizer, outputs are a c++ strings that are not necessarily valid utf-8 encodings. Default pybind11 casting uses strict utf-8 decoding. We relax the decoding using 'ignore' argument. Test Plan: https://www.internalfb.com/intern/testinfra/testrun/4503599786612705 Reviewed By: Nayef211 Differential Revision: D43970697 fbshipit-source-id: 872c1ea41870d885ad52c39a93735afa43020525

facebook-github-bot · 2023-03-22T01:56:32Z

This pull request was exported from Phabricator. Differential Revision: D43970697

… python str. (pytorch#97282) Summary: Pull Request resolved: pytorch#97282 X-link: pytorch/text#2126 When language models use c++ tokenizer, outputs are a c++ strings that are not necessarily valid utf-8 encodings. Default pybind11 casting uses strict utf-8 decoding. We relax the decoding using 'ignore' argument. Test Plan: https://www.internalfb.com/intern/testinfra/testrun/4503599786612705 Reviewed By: Nayef211 Differential Revision: D43970697 fbshipit-source-id: d54a7c527d702bec37544442f9ce6e2d441ddc47

facebook-github-bot · 2023-03-22T20:05:14Z

This pull request was exported from Phabricator. Differential Revision: D43970697

shuminghu · 2023-03-23T01:17:14Z

@pytorchbot merge

pytorchmergebot · 2023-03-23T01:19:03Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

… python str. (#97282) Summary: When language models use c++ tokenizer, outputs are a c++ strings that are not necessarily valid utf-8 encodings. Default pybind11 casting uses strict utf-8 decoding. We relax the decoding using 'ignore' argument. Test Plan: https://www.internalfb.com/intern/testinfra/testrun/6473924609918070 Reviewed By: Nayef211 Differential Revision: D43970697 Pull Request resolved: pytorch/pytorch#97282 Approved by: https://github.com/davidberard98

Summary: Binding and test to make sure we can use 'ignore' option for utf-8 decoding added to pytorch in D43970697( pytorch/pytorch#97282). Reviewed By: Nayef211 Differential Revision: D44315169 fbshipit-source-id: fa0f2fb02fb65c389743e32455dd5875d4e0324a

…ytorch#2128) Summary: Pull Request resolved: pytorch#2128 Binding and test to make sure we can use 'ignore' option for utf-8 decoding added to pytorch in D43970697( pytorch/pytorch#97282). Reviewed By: Nayef211 Differential Revision: D44315169 fbshipit-source-id: a4a60549c8945c8e8594f4f2cc9e7c99d105128d

…2128) Summary: Pull Request resolved: #2128 Binding and test to make sure we can use 'ignore' option for utf-8 decoding added to pytorch in D43970697( pytorch/pytorch#97282). Reviewed By: Nayef211 Differential Revision: D44315169 fbshipit-source-id: d42fcacafd429cf586c631faf826abc172b173d3

…ytorch#2128) Summary: Pull Request resolved: pytorch#2128 Binding and test to make sure we can use 'ignore' option for utf-8 decoding added to pytorch in D43970697( pytorch/pytorch#97282). Reviewed By: Nayef211 Differential Revision: D44315169 fbshipit-source-id: d42fcacafd429cf586c631faf826abc172b173d3

#2134) * Optionally ignore utf-8 decoding error for scripted C++ tokenizers. (#2128) Summary: Pull Request resolved: #2128 Binding and test to make sure we can use 'ignore' option for utf-8 decoding added to pytorch in D43970697( pytorch/pytorch#97282). Reviewed By: Nayef211 Differential Revision: D44315169 fbshipit-source-id: d42fcacafd429cf586c631faf826abc172b173d3 * Linter fixes --------- Co-authored-by: Shuming Hu <smhu@meta.com>

pytorch-bot bot added the release notes: jit release notes category label Mar 21, 2023

facebook-github-bot added the fb-exported label Mar 21, 2023

shuminghu force-pushed the export-D43970697 branch from dd7a4c0 to f73b6b2 Compare March 21, 2023 22:32

shuminghu force-pushed the export-D43970697 branch from f73b6b2 to 1a8a1c4 Compare March 21, 2023 22:39

shuminghu self-assigned this Mar 21, 2023

shuminghu requested a review from davidberard98 March 21, 2023 22:54

davidberard98 approved these changes Mar 22, 2023

View reviewed changes

shuminghu force-pushed the export-D43970697 branch from 1a8a1c4 to 96ce7aa Compare March 22, 2023 01:40

shuminghu force-pushed the export-D43970697 branch from 96ce7aa to adfa1ef Compare March 22, 2023 01:48

shuminghu force-pushed the export-D43970697 branch from adfa1ef to 59cf210 Compare March 22, 2023 01:56

shuminghu force-pushed the export-D43970697 branch from 59cf210 to 237daaa Compare March 22, 2023 20:05

davidberard98 added the ciflow/trunk Trigger trunk jobs on your pull request label Mar 22, 2023

pytorchmergebot added the Merged label Mar 23, 2023

pytorchmergebot closed this in b45880c Mar 23, 2023

shuminghu mentioned this pull request Mar 27, 2023

Optionally ignore utf-8 decoding error for scripted C++ tokenizers. pytorch/text#2128

Closed

Nayef211 mentioned this pull request Mar 29, 2023

Optionally ignore utf-8 decoding error for scripted C++ tokenizers. (… pytorch/text#2134

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optionally ignore utf-8 decoding error when converting std::string to python str. #97282

Optionally ignore utf-8 decoding error when converting std::string to python str. #97282

shuminghu commented Mar 21, 2023

pytorch-bot bot commented Mar 21, 2023 •

edited

linux-foundation-easycla bot commented Mar 21, 2023 •

edited

facebook-github-bot commented Mar 21, 2023

facebook-github-bot commented Mar 21, 2023

facebook-github-bot commented Mar 21, 2023

davidberard98 left a comment

davidberard98 Mar 22, 2023

shuminghu Mar 22, 2023

davidberard98 Mar 22, 2023

shuminghu Mar 22, 2023 •

edited

davidberard98 Mar 22, 2023

shuminghu Mar 22, 2023

shuminghu Mar 22, 2023

shuminghu commented Mar 22, 2023

facebook-github-bot commented Mar 22, 2023

facebook-github-bot commented Mar 22, 2023

facebook-github-bot commented Mar 22, 2023

facebook-github-bot commented Mar 22, 2023

shuminghu commented Mar 23, 2023

pytorchmergebot commented Mar 23, 2023

Optionally ignore utf-8 decoding error when converting std::string to python str. #97282

Optionally ignore utf-8 decoding error when converting std::string to python str. #97282

Conversation

shuminghu commented Mar 21, 2023

pytorch-bot bot commented Mar 21, 2023 • edited

🔗 Helpful Links

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

⏳ No Failures, 5 Pending

linux-foundation-easycla bot commented Mar 21, 2023 • edited

facebook-github-bot commented Mar 21, 2023

facebook-github-bot commented Mar 21, 2023

facebook-github-bot commented Mar 21, 2023

davidberard98 left a comment

Choose a reason for hiding this comment

davidberard98 Mar 22, 2023

Choose a reason for hiding this comment

shuminghu Mar 22, 2023

Choose a reason for hiding this comment

davidberard98 Mar 22, 2023

Choose a reason for hiding this comment

shuminghu Mar 22, 2023 • edited

Choose a reason for hiding this comment

davidberard98 Mar 22, 2023

Choose a reason for hiding this comment

shuminghu Mar 22, 2023

Choose a reason for hiding this comment

shuminghu Mar 22, 2023

Choose a reason for hiding this comment

shuminghu commented Mar 22, 2023

facebook-github-bot commented Mar 22, 2023

facebook-github-bot commented Mar 22, 2023

facebook-github-bot commented Mar 22, 2023

facebook-github-bot commented Mar 22, 2023

shuminghu commented Mar 23, 2023

pytorchmergebot commented Mar 23, 2023

Merge started

pytorch-bot bot commented Mar 21, 2023 •

edited

linux-foundation-easycla bot commented Mar 21, 2023 •

edited

shuminghu Mar 22, 2023 •

edited