-
Notifications
You must be signed in to change notification settings - Fork 811
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
switch to_ivalue to __prepare_scriptable__ #1080
Changes from all commits
0b6c24d
9eb20d9
40ff363
edf80f8
129887f
1396901
fff1adb
f9a7d5b
c6f4236
1fb69bd
6a756d5
859e363
4f48369
8809bd5
1e1f88a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -94,14 +94,16 @@ def test_BasicEnglishNormalize(self): | |
basic_eng_norm = basic_english_normalize() | ||
experimental_eager_tokens = basic_eng_norm(test_sample) | ||
|
||
jit_basic_eng_norm = torch.jit.script(basic_eng_norm.to_ivalue()) | ||
jit_basic_eng_norm = torch.jit.script(basic_eng_norm) | ||
experimental_jit_tokens = jit_basic_eng_norm(test_sample) | ||
|
||
basic_english_tokenizer = data.get_tokenizer("basic_english") | ||
eager_tokens = basic_english_tokenizer(test_sample) | ||
|
||
assert not basic_eng_norm.is_jitable | ||
assert basic_eng_norm.to_ivalue().is_jitable | ||
# Call the __prepare_scriptable__() func and convert the building block to the torbhind version | ||
# Not expect users to use the torchbind version on eager mode but still need a CI test here. | ||
assert basic_eng_norm.__prepare_scriptable__().is_jitable | ||
|
||
self.assertEqual(experimental_jit_tokens, ref_results) | ||
self.assertEqual(eager_tokens, ref_results) | ||
|
@@ -121,7 +123,9 @@ def test_basicEnglishNormalize_load_and_save(self): | |
|
||
with self.subTest('torchscript'): | ||
save_path = os.path.join(self.test_dir, 'ben_torchscrip.pt') | ||
ben = basic_english_normalize().to_ivalue() | ||
# Call the __prepare_scriptable__() func and convert the building block to the torbhind version | ||
# Not expect users to use the torchbind version on eager mode but still need a CI test here. | ||
ben = basic_english_normalize().__prepare_scriptable__() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd interact with this via torch.jit.script. These methods are not meant to be called directly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See the comment above. |
||
torch.save(ben, save_path) | ||
loaded_ben = torch.load(save_path) | ||
self.assertEqual(loaded_ben(test_sample), ref_results) | ||
|
@@ -149,11 +153,13 @@ def test_RegexTokenizer(self): | |
r_tokenizer = regex_tokenizer(patterns_list) | ||
eager_tokens = r_tokenizer(test_sample) | ||
|
||
jit_r_tokenizer = torch.jit.script(r_tokenizer.to_ivalue()) | ||
jit_r_tokenizer = torch.jit.script(r_tokenizer) | ||
jit_tokens = jit_r_tokenizer(test_sample) | ||
|
||
assert not r_tokenizer.is_jitable | ||
assert r_tokenizer.to_ivalue().is_jitable | ||
# Call the __prepare_scriptable__() func and convert the building block to the torbhind version | ||
# Not expect users to use the torchbind version on eager mode but still need a CI test here. | ||
assert r_tokenizer.__prepare_scriptable__().is_jitable | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See the comment above. |
||
|
||
self.assertEqual(eager_tokens, ref_results) | ||
self.assertEqual(jit_tokens, ref_results) | ||
|
@@ -186,7 +192,9 @@ def test_load_and_save(self): | |
|
||
with self.subTest('torchscript'): | ||
save_path = os.path.join(self.test_dir, 'regex_torchscript.pt') | ||
tokenizer = regex_tokenizer(patterns_list).to_ivalue() | ||
# Call the __prepare_scriptable__() func and convert the building block to the torbhind version | ||
# Not expect users to use the torchbind version on eager mode but still need a CI test here. | ||
tokenizer = regex_tokenizer(patterns_list).__prepare_scriptable__() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But we shouldn't have to JIT anything just to save it, but if we want to exercise saving a JIT'd model then we should call torch.jit.script first There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See the comment above. We want to save a torchbind version model here, not exactly the JIT mode. |
||
torch.save(tokenizer, save_path) | ||
loaded_tokenizer = torch.load(save_path) | ||
results = loaded_tokenizer(test_sample) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd interact with this via torch.jit.script. These methods are not meant to be called directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we want to check if the building block backed by torchbind is jitable. Although we don't recommend the torchbind version for eager mode, I think we still need a test to cover it. By calling
torch.jit.script
, we are testing the jit mode, which is not the same thing with the torchbind building block.Same thing for the pickle support.