Add dpt-hybrid support
#20645
Add dpt-hybrid support
#20645
Conversation
|
The documentation is not available anymore as the PR was closed or merged. |
patrickvonplaten
left a comment
patrickvonplaten
left a comment
There was a problem hiding this comment.
Looks good to me! Let's please wait though until @sgugger has approved as well :-)
Co-authored-by: Patrick von Platen <[email protected]>
| pooler_output=pooled_output, | ||
| hidden_states=encoder_outputs.hidden_states, | ||
| attentions=encoder_outputs.attentions, | ||
| intermediate_activations=embedding_output.intermediate_activations, |
There was a problem hiding this comment.
This is a breaking change, as users who are currently using DPTModel expect 4 keys in the output class => this will now be 5.
There was a problem hiding this comment.
Only if those are all not None however.
| logger.info("Initializing the config with a `BiT` backbone.") | ||
| self.backbone_config = BitConfig(**backbone_config) | ||
| elif isinstance(backbone_config, PretrainedConfig): | ||
| self.backbone_config = backbone_config |
There was a problem hiding this comment.
Not sure this model works with other backbones, but sure let's support it :D
sgugger
left a comment
sgugger
left a comment
There was a problem hiding this comment.
Not a huge fan of the modularity introduced in the modeling code, but okay in this case since it's all in the same paper.
Left a comment on the config param introduced, other than that it should be good to merge soon!
| pooler_output=pooled_output, | ||
| hidden_states=encoder_outputs.hidden_states, | ||
| attentions=encoder_outputs.attentions, | ||
| intermediate_activations=embedding_output.intermediate_activations, |
There was a problem hiding this comment.
Only if those are all not None however.
Co-authored-by: Sylvain Gugger <[email protected]>
…kada/transformers into add-dpt-hybrid-support
sgugger
left a comment
sgugger
left a comment
There was a problem hiding this comment.
You'll just need to adapt the checkpoint online with the new config arg and then should be good to merge!
|
Thanks a bunch! Fortunately it was already on the config file :D https://huggingface.co/Intel/dpt-hybrid-midas/blob/main/config.json#L277 but will open a PR to remove the |
|
The config file has been modified, merging! |
* add `dpt-hybrid` support * refactor * final changes, all tests pass * final cleanups * final changes * Apply suggestions from code review Co-authored-by: Patrick von Platen <[email protected]> * fix docstring * fix typo * change `vit_hybrid` to `hybrid` * replace dataclass * add docstring * move dataclasses * fix test * add `PretrainedConfig` support for `backbone_config` * fix docstring * Apply suggestions from code review Co-authored-by: Sylvain Gugger <[email protected]> * remove `embedding_type` and replace it by `is_hybrid` Co-authored-by: Patrick von Platen <[email protected]> Co-authored-by: Sylvain Gugger <[email protected]>
5 participants
What does this PR do?
Adds
DPT-hybridsupport intransformersCurrently only DPT is supported. This PR leverages
AutoBackbonefrom @NielsRogge to replace the embedding layer fromDPTto supportDPT-hybridFixes #20435
Model weights: https://huggingface.co/Intel/dpt-hybrid-midas