Align backbone stage selection with out_indices & out_features #27606
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. |
amyeroberts
force-pushed
the
fix-backbone-forward-bug
branch
from
6d5df96 to
2e1efaf
Compare
| if stage in self.out_features: | ||
| feature_maps += (hidden_states[idx],) | ||
| for idx in self.out_indices: | ||
| feature_maps += (hidden_states[idx],) |
There was a problem hiding this comment.
do we not want to check the index here as well?
| hidden_state = self.hidden_states_norms[stage](hidden_state) | ||
| feature_maps += (hidden_state,) | ||
| for stage in self.out_features: | ||
| idx = self.stage_names.index(stage) |
There was a problem hiding this comment.
I heard that .index is only available starting python 3.6 unless the dict was always ordered
There was a problem hiding this comment.
stage_names is a list and we only support py >= 3.8 so I think it's safe :)
@ArthurZucker Sure! It's both - a choice and a current bug. The choice is whether we allow passing in different orders and duplicates and the bug is whether this is reflected. At the moment I can pass in duplicates, out-of-order etc. but it won't be reflected in the returned stages. Another option is for input verification where we raise an error if the user chooses |
|
@ArthurZucker Sorry to flip-flop. I've thought a bit more and concluded that not allowing repetitions & different orders when setting
I'm going to update the PR to add these checks + relevant tests instead |
amyeroberts
changed the title
amyeroberts
changed the title
amyeroberts
changed the title
amyeroberts
force-pushed
the
fix-backbone-forward-bug
branch
from
389668c to
f5b5db4
Compare
|
@ArthurZucker There's isn't any proper documentation for the backbones atm - this is being added in #27456. I've added notes about the restrictions in the docstrings |
amyeroberts
force-pushed
the
fix-backbone-forward-bug
branch
from
889b42e to
5c194eb
Compare
…ngface#27606) * Iteratre over out_features instead of stage_names * Update for all backbones * Add tests * Fix * Align timm backbone behaviour with other backbones * Fix tests * Stricter checks on set out_features and out_indices * Revert back stage selection logic * Remove out-of-order logic * Document restriction in docstrings
What does this PR do?
This PR adds a set of input verification for the
out_featuresandout_indicesarguments for backbones, making sure that any accepted values align with the returned model outputs.More details
out_featuresandout_indicesare used to specify which blocks' attentions are returned by the Backbone classes.The following can currently be passed in
out_features:["stage5", "stage2", "stage4"]["stage3", "stage3"]However, this is will not be reflected in the returned feature maps on a forward pass e.g. here for ResNet. The feature maps are selected by iterating over the stage_names (ordered list of all stages in the backbone) and returning those that have their name in
out_featuresand so are in stage-order and will only be selected once.There is also a misalignment between the TimmBackbone and transformers backbones - as timm will automatically take the set of indices (removing duplicates) whereas transformers will keep them in the
out_indicesattribute.Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.