This repository has been archived by the owner on Jul 1, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 276
Make blocks attachable automatically when needed #461
Closed
Closed
Conversation
This file contains 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
facebook-github-bot
added
CLA Signed
This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
fb-exported
labels
Mar 31, 2020
This pull request was exported from Phabricator. Differential Revision: D20714865 |
mannatsingh
added a commit
to mannatsingh/ClassyVision
that referenced
this pull request
Apr 1, 2020
Summary: Pull Request resolved: facebookresearch#461 I was frustrated by the fact that I needed to change the code for every model if I needed to be able to attach heads to it - something we need to do with most trained models. Ideally, users should be able to write their models without writing anything classy vision specific and be able to attach heads. I've made a couple of diffs which get us there, wanted to see what everyone thought about them. Please look at the second diff to see the end result. In this diff, I make the following changes - - `build_attachable_block` becomes a private function (`_build_attachable_block`), and models don't need to call it anymore. - Instead, when someone tries to attach to a module called `my_block`, we recursively search for it and wrap it by a `ClassyBlock` on the fly - Updated `get_classy_state` and `set_classy_state` to be compatible with the changes - Users can attach heads to any block which has a unique name (like `block3-2`) - `models_classy_model_test` wasn't being run internally; renamed `classy_block_test` to `models_classy_block_test` - Added additional test cases to test the changes NOTE: This breaks all checkpoints since the model definitions have changed. Differential Revision: D20714865 fbshipit-source-id: 95c1ce4655663538919ca28f77f554ea67570edb
mannatsingh
force-pushed
the
export-D20714865
branch
from
April 1, 2020 00:05
b9f2305
to
061e082
Compare
This pull request was exported from Phabricator. Differential Revision: D20714865 |
mannatsingh
added a commit
to mannatsingh/ClassyVision
that referenced
this pull request
Apr 1, 2020
Summary: Pull Request resolved: facebookresearch#461 I was frustrated by the fact that I needed to change the code for every model if I needed to be able to attach heads to it - something we need to do with most trained models. Ideally, users should be able to write their models without writing anything classy vision specific and be able to attach heads. I've made a couple of diffs which get us there, wanted to see what everyone thought about them. Please look at the second diff to see the end result. In this diff, I make the following changes - - `build_attachable_block` becomes a private function (`_build_attachable_block`), and models don't need to call it anymore. - Removed `_should_cache_output` and `set_cache_output` from `ClassyBlock` - Added a redundant `_attachable_block_names` attribute - this is needed for reading the block names inside torch script (`_attachable_blocks` is inaccessible) - T64918869 - Instead, when someone tries to attach to a module called `my_block`, we recursively search for it and wrap it by a `ClassyBlock` on the fly - Updated `get_classy_state` and `set_classy_state` to be compatible with the changes - Users can attach heads to any block which has a unique name (like `block3-2`) - `models_classy_model_test` wasn't being run internally; renamed `classy_block_test` to `models_classy_block_test` - Added additional test cases to test the changes NOTE: This breaks all checkpoints since the model definitions have changed. We still handle old checkpoints for `ResNeXt` models to allow for a smoother transition - T61141249 Differential Revision: D20714865 fbshipit-source-id: f48e768aede1c10d25754f0fad0f24ccac9a1503
mannatsingh
force-pushed
the
export-D20714865
branch
from
April 1, 2020 21:23
061e082
to
3dffb1b
Compare
This pull request was exported from Phabricator. Differential Revision: D20714865 |
mannatsingh
added a commit
to mannatsingh/ClassyVision
that referenced
this pull request
Apr 1, 2020
Summary: Pull Request resolved: facebookresearch#461 I was frustrated by the fact that I needed to change the code for every model if I needed to be able to attach heads to it - something we need to do with most trained models. Ideally, users should be able to write their models without writing anything classy vision specific and be able to attach heads. I've made a couple of diffs which get us there, wanted to see what everyone thought about them. Please look at the second diff to see the end result. In this diff, I make the following changes - - `build_attachable_block` becomes a private function (`_build_attachable_block`), and models don't need to call it anymore. - Removed `_should_cache_output` and `set_cache_output` from `ClassyBlock` - Added a redundant `_attachable_block_names` attribute - this is needed for reading the block names inside torch script (`_attachable_blocks` is inaccessible) - T64918869 - Instead, when someone tries to attach to a module called `my_block`, we recursively search for it and wrap it by a `ClassyBlock` on the fly - Updated `get_classy_state` and `set_classy_state` to be compatible with the changes - Users can attach heads to any block which has a unique name (like `block3-2`) - `models_classy_model_test` wasn't being run internally; renamed `classy_block_test` to `models_classy_block_test` - Added additional test cases to test the changes NOTE: This breaks all checkpoints since the model definitions have changed. We still handle old checkpoints for `ResNeXt` models to allow for a smoother transition - T61141249 Reviewed By: vreis Differential Revision: D20714865 fbshipit-source-id: 993a841b7190d5fabd751d8cca25381dcd93d898
mannatsingh
force-pushed
the
export-D20714865
branch
from
April 1, 2020 21:42
3dffb1b
to
3b278dc
Compare
This pull request was exported from Phabricator. Differential Revision: D20714865 |
Summary: Pull Request resolved: facebookresearch#461 I was frustrated by the fact that I needed to change the code for every model if I needed to be able to attach heads to it - something we need to do with most trained models. Ideally, users should be able to write their models without writing anything classy vision specific and be able to attach heads. I've made a couple of diffs which get us there, wanted to see what everyone thought about them. Please look at the second diff to see the end result. In this diff, I make the following changes - - `build_attachable_block` becomes a private function (`_build_attachable_block`), and models don't need to call it anymore. - Removed `_should_cache_output` and `set_cache_output` from `ClassyBlock` - Added a redundant `_attachable_block_names` attribute - this is needed for reading the block names inside torch script (`_attachable_blocks` is inaccessible) - T64918869 - Instead, when someone tries to attach to a module called `my_block`, we recursively search for it and wrap it by a `ClassyBlock` on the fly - Updated `get_classy_state` and `set_classy_state` to be compatible with the changes - Users can attach heads to any block which has a unique name (like `block3-2`) - `models_classy_model_test` wasn't being run internally; renamed `classy_block_test` to `models_classy_block_test` - Added additional test cases to test the changes NOTE: This breaks all checkpoints since the model definitions have changed. We still handle old checkpoints for `ResNeXt` models to allow for a smoother transition - T61141249 Reviewed By: vreis Differential Revision: D20714865 fbshipit-source-id: d3cd867886ad857efe31442458da761418a51d6d
mannatsingh
force-pushed
the
export-D20714865
branch
from
April 8, 2020 22:21
3b278dc
to
761da95
Compare
This pull request was exported from Phabricator. Differential Revision: D20714865 |
facebook-github-bot
pushed a commit
that referenced
this pull request
Apr 9, 2020
Summary: Pull Request resolved: #461 I was frustrated by the fact that I needed to change the code for every model if I needed to be able to attach heads to it - something we need to do with most trained models. Ideally, users should be able to write their models without writing anything classy vision specific and be able to attach heads. I've made a couple of diffs which get us there, wanted to see what everyone thought about them. Please look at the second diff to see the end result. In this diff, I make the following changes - - `build_attachable_block` becomes a private function (`_build_attachable_block`), and models don't need to call it anymore. - Removed `_should_cache_output` and `set_cache_output` from `ClassyBlock` - Added a redundant `_attachable_block_names` attribute - this is needed for reading the block names inside torch script (`_attachable_blocks` is inaccessible) - T64918869 - Instead, when someone tries to attach to a module called `my_block`, we recursively search for it and wrap it by a `ClassyBlock` on the fly - Updated `get_classy_state` and `set_classy_state` to be compatible with the changes - Users can attach heads to any block which has a unique name (like `block3-2`) - `models_classy_model_test` wasn't being run internally; renamed `classy_block_test` to `models_classy_block_test` - Added additional test cases to test the changes NOTE: This breaks all checkpoints since the model definitions have changed. We still handle old checkpoints for `ResNeXt` models to allow for a smoother transition - T61141249 Reviewed By: vreis Differential Revision: D20714865 fbshipit-source-id: f8fa269b5deddbdd77461047850e7050b1bc921a
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
CLA Signed
This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
fb-exported
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.
Summary:
I was frustrated by the fact that I needed to change the code for every model if I needed to be able to attach heads to it - something we need to do with most trained models.
Ideally, users should be able to write their models without writing anything classy vision specific and be able to attach heads.
I've made a couple of diffs which get us there, wanted to see what everyone thought about them. Please look at the second diff to see the end result.
In this diff, I make the following changes -
build_attachable_block
becomes a private function (_build_attachable_block
), and models don't need to call it anymore.my_block
, we recursively search for it and wrap it by aClassyBlock
on the flyget_classy_state
andset_classy_state
to be compatible with the changesblock3-2
)models_classy_model_test
wasn't being run internally; renamedclassy_block_test
tomodels_classy_block_test
NOTE: This breaks all checkpoints since the model definitions have changed.
Differential Revision: D20714865