Add pad transform, string to int transform #1683
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: Rather than raise an exception whenever head_dim != 64, we can just infer the scaling value and continue to provide a warning. Also add an assertion in case embed_dim is not a multiple of num_heads (in which case forward will break). Reviewed By: parmeet Differential Revision: D32193989 fbshipit-source-id: 30f68c55f3ec37932252c77c355ae55b8bf34ded
Test plan: Add unit tests ``` python3 -m pytest -v -k 'pad_transform or str_to_int' ====================================================================================================== test session starts ======================================================================================================= platform linux -- Python 3.9.12, pytest-6.2.5, py-1.11.0, pluggy-1.0.0 -- /data/home/ebs/miniconda/envs/torchtext/bin/python3 cachedir: .pytest_cache rootdir: /data/home/ebs/torchtext, configfile: pytest.ini, testpaths: test/ plugins: pythonpath-0.7.4, cov-3.0.0 collected 1091 items / 1087 deselected / 4 selected test/test_transforms.py::TestTransforms::test_pad_transform PASSED [ 25%] test/test_transforms.py::TestTransforms::test_pad_transform_jit PASSED [ 50%] test/test_transforms.py::TestTransforms::test_str_to_int_transform PASSED [ 75%] test/test_transforms.py::TestTransforms::test_str_to_int_transform_jit PASSED [100%] ============================================================================================== 4 passed, 1087 deselected in 10.57s =============================================================================================== ```
parmeet
reviewed
Apr 18, 2022
torchtext/transforms.py
Outdated
Comment on lines
226
to
237
| class PadTransform(Module): | ||
| def __init__(self, max_length: int, pad_value: int): | ||
| super().__init__() | ||
| self.max_length = max_length | ||
| self.pad_value = pad_value | ||
|
|
||
| def forward(self, x: torch.Tensor) -> torch.Tensor: | ||
| max_encoded_length = x.size(-1) | ||
| if max_encoded_length < self.max_length: | ||
| pad_amount = self.max_length - max_encoded_length | ||
| x = torch.nn.functional.pad(x, (0, pad_amount), value=self.pad_value) | ||
| return x |
There was a problem hiding this comment.
could you please add doc-strings for __init__ and forward methods? Also update the the rst file to update the documentation.
| return x | ||
|
|
||
|
|
||
| class StrToIntTransform(Module): |
There was a problem hiding this comment.
Same comment for doc-strings :)
mthrok
reviewed
Apr 20, 2022
docs/source/transforms.rst
Outdated
| @@ -71,3 +71,17 @@ Sequential | |||
| .. autoclass:: Sequential | |||
|
|
|||
| .. automethod:: forward | |||
|
|
|||
| PadTransform | |||
| ---------- | |||
There was a problem hiding this comment.
Suggested change
| ---------- | |
| ------------ |
mthrok
reviewed
Apr 20, 2022
docs/source/transforms.rst
Outdated
| .. automethod:: forward | ||
|
|
||
| StrToIntTransform | ||
| ---------- |
There was a problem hiding this comment.
Suggested change
| ---------- | |
| ----------------- |
mthrok
reviewed
Apr 20, 2022
| @@ -205,6 +205,76 @@ def test_add_token(self): | |||
| def test_add_token_jit(self): | |||
| self._add_token(test_scripting=True) | |||
|
|
|||
| def _pad_transform(self, test_scripting): | |||
There was a problem hiding this comment.
Can you add a brief description of what this test does?
In the form of "(under some condition), an API produces a result that satisfies this property"
parmeet
approved these changes
Apr 21, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Test plan:
Add unit tests