[go: nahoru, domu]

Skip to content
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

Update timm to 1.* version and support more encoders #885

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

qubvel
Copy link
Owner
@qubvel qubvel commented Jun 8, 2024

Make most of the models from timm compatible with decoders (including transformers and convnext models)

@qubvel qubvel marked this pull request as draft June 8, 2024 23:32
Copy link
@wouterzwerink wouterzwerink left a comment

Choose a reason for hiding this comment

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

Hi! I've left some suggestions that fixed the tests on my side, since I am looking to use segmentation_models_pytorch with timm>1.0

in_channels: int = 3,
depth: int = 5,
output_stride: int = 32,
out_indices: Optional[List[int]] = None,

Choose a reason for hiding this comment

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

Suggested change
out_indices: Optional[List[int]] = None,
out_indices: Union[str, List[int]] = "first",

Comment on lines +56 to +57
if "encoder_indices" in kwargs and kwargs["encoder_indices"] is None:
kwargs["encoder_indices"] = "first"

Choose a reason for hiding this comment

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

Suggested change
if "encoder_indices" in kwargs and kwargs["encoder_indices"] is None:
kwargs["encoder_indices"] = "first"
if "out_indices" in kwargs and kwargs["out_indices"] is None:
kwargs["out_indices"] = "first"

While it's called encoder_indices in the function select_feature_indices, in TimmUniversalEncoder it is still called out_indices

Comment on lines +69 to +73
encoder_indices = kwargs.pop("encoder_indices", None)
if encoder_indices is not None:
logger.warning(
"Argument `encoder_indices` is supported only for `tu-` encoders (Timm) and will be ignored."
)

Choose a reason for hiding this comment

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

Suggested change
encoder_indices = kwargs.pop("encoder_indices", None)
if encoder_indices is not None:
logger.warning(
"Argument `encoder_indices` is supported only for `tu-` encoders (Timm) and will be ignored."
)
out_indices = kwargs.pop("encoder_indices", None)
if out_indices is not None:
logger.warning(
"Argument `out_indices ` is supported only for `tu-` encoders (Timm) and will be ignored."
)

@qubvel
Copy link
Owner Author
qubvel commented Jun 24, 2024

Hi @wouterzwerink thanks for the review!
I am working on it, it requires a bit more in depth analysis to make most of the models compatible and configurable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants