Perceiver interpolate position embedding #30979
Conversation
| inputs: torch.LongTensor, | ||
| pos: Optional[torch.Tensor] = None, | ||
| network_input_is_1d: bool = True, | ||
| interpolate_pos_encoding: bool = None, |
There was a problem hiding this comment.
The argument doesn't appear to be used in this method?
There was a problem hiding this comment.
When I ran the test, TextPreprocesor and AudioPreprocessor didn't fail but only MutimodalPreprocessor did so I missed those. Just added.
There was a problem hiding this comment.
interpolate_pos_encoding still isn't used in this method., or the one for PerceiverAudioPreprocessor. Is this for providing a consistent signature?
…nce only Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>
9a55e50 to
2891e0d
Compare
|
Thanks for the review, I have addressed the comments. Turns out after working on the no longer needed |
| inputs: torch.LongTensor, | ||
| pos: Optional[torch.Tensor] = None, | ||
| network_input_is_1d: bool = True, | ||
| interpolate_pos_encoding: bool = None, |
There was a problem hiding this comment.
interpolate_pos_encoding still isn't used in this method., or the one for PerceiverAudioPreprocessor. Is this for providing a consistent signature?
| if self.input_preprocessor is not None: | ||
| inputs, modality_sizes, inputs_without_pos = self.input_preprocessor(inputs) | ||
| inputs, modality_sizes, inputs_without_pos = self.input_preprocessor( | ||
| inputs, interpolate_pos_encoding=interpolate_pos_encoding | ||
| ) |
There was a problem hiding this comment.
@amyeroberts since self.input_processor here could also be either TextPreprocessor or MultimodalPreprosessor, it would complain got unexpected keyword argument if it was not added to the forward() of those two corresponding methods. Alternatively, maybe we could add another condition here to call self.input_processor with just two arguments unless it is ImagePreprocessor?
There was a problem hiding this comment.
No, that's OK, I just wanted to make sure that this was deliberate
What does this PR do?
Add interpolate position embedding to Perceiver #30579. Not really sure how to test the correctness of output value since it does change the entire forward pass. However, correctness of output shape is pretty straight forward.
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.
@amyeroberts