Skip to content

replace query_recursively with pytree implementation #6434

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

Merged
merged 3 commits into from
Aug 18, 2022

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Aug 17, 2022

Addresses #6408 (comment).

Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

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

Thanks @pmeier

@@ -361,7 +352,7 @@ def __init__(
def forward(self, *inputs: Any) -> Any:
sample = inputs if len(inputs) > 1 else inputs[0]

id, image = self._extract_image(sample)
sample_flat, sample_spec, idx_flat, image = self._flatten(sample)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I appologize if I'm missing the context, If I understand correctly the code, you fetch the first image from sample with _flatten and below in the code the image is transformed and written back to sample.
Why do we transform only the first image ?

Copy link
Collaborator

@vfdev-5 vfdev-5 Aug 17, 2022

Choose a reason for hiding this comment

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

More generally question, why can't we restructure the implementation such that we use forward to inspect sample and ensure that only images (and/or only one image in sample) go further into _transform. In _transform we do the necessary AA ops. What do you think ?

Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@pmeier pmeier merged commit 2a0eea8 into pytorch:main Aug 18, 2022
@pmeier pmeier deleted the query-recursively branch August 18, 2022 13:18
facebook-github-bot pushed a commit that referenced this pull request Aug 25, 2022
Summary:
* replace query_recursively with pytree implementation

* simplify

Reviewed By: datumbox

Differential Revision: D39013677

fbshipit-source-id: ac63607513bb5457ead65e798a8704c6341c5c50

Co-authored-by: vfdev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants