Skip to content

Conversation

vfdev-5
Copy link
Collaborator

@vfdev-5 vfdev-5 commented Oct 7, 2021

Description:

  • Use FX feature extractor for segm model

Related to #4540

cc @datumbox

@datumbox
Copy link
Contributor

datumbox commented Oct 7, 2021

@vfdev-5 Thanks for the PR.

Adding the extra parameter will introduce twice as many tests for each model which will slow things down. As discussed at #4540 (comment), which solution we use to extract the features seems an internal detail for the models that we can easily adapt if the FX requires changes. Moreover given that the module models.feature_extraction is public, we would need to resolve the issue in a BC way anyway. I would be in favour of just replacing the old method on the PR but let me know your thoughts.

@vfdev-5 vfdev-5 force-pushed the segm-model-with-fe branch from 94bf3bb to 21b0ff8 Compare October 7, 2021 15:18
@vfdev-5
Copy link
Collaborator Author

vfdev-5 commented Oct 7, 2021

@datumbox OK, sounds good to remove use_fe option. Removed it.

Copy link
Member

@fmassa fmassa 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 a lot!

Copy link
Contributor

@prabhat00155 prabhat00155 left a comment

Choose a reason for hiding this comment

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

Thanks @vfdev-5!

@datumbox datumbox merged commit 4614cf9 into pytorch:main Oct 7, 2021
@vfdev-5 vfdev-5 deleted the segm-model-with-fe branch October 8, 2021 08:44
facebook-github-bot pushed a commit that referenced this pull request Oct 8, 2021
Summary:
* Use FX feature extractor for segm model

* Removed use_fe option

Reviewed By: NicolasHug

Differential Revision: D31505556

fbshipit-source-id: ff7b12aabab5363dd0ae10a03aaf008e7497946e
mszhanyi pushed a commit to mszhanyi/vision that referenced this pull request Oct 19, 2021
* Use FX feature extractor for segm model

* Removed use_fe option
cyyever pushed a commit to cyyever/vision that referenced this pull request Nov 16, 2021
* Use FX feature extractor for segm model

* Removed use_fe option
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