Skip to content

Conversation

@arun477
Copy link
Contributor

@arun477 arun477 commented Apr 27, 2024

transformers PreTrainedModel._tie_encoder_decoder_weight in DenoisingAutoEncoderLoss requires positional argument base_encoder_name.

image
image
image

@@ -114,7 +114,7 @@ def __init__(self, model: SentenceTransformer, decoder_name_or_path: str = None,
)
decoder_base_model_prefix = self.decoder.base_model_prefix
PreTrainedModel._tie_encoder_decoder_weights(
Copy link
Member

@tomaarsen tomaarsen May 1, 2024

Choose a reason for hiding this comment

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

Hello!

Well spotted; I see this has been introduced in transformers v4.40.0 here. Because I'd rather not increment the minimum transformers all the way to v4.44.0 yet (I imagine that might cause some compatibility problems for people), I propose an alternative solution:

try:
    # Compatibility with transformers <4.40.0
    PreTrainedModel._tie_encoder_decoder_weights(
        model[0].auto_model, self.decoder._modules[decoder_base_model_prefix], self.decoder.base_model_prefix
    )
except TypeError:
    # Compatibility with transformers >=4.40.0
    PreTrainedModel._tie_encoder_decoder_weights(
        model[0].auto_model, self.decoder._modules[decoder_base_model_prefix], self.decoder.base_model_prefix, encoder_name_or_path
    )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! @tomaarsen makes sense. It has been updated and now works fine!

@tomaarsen tomaarsen merged commit f1b923b into huggingface:master May 8, 2024
@tomaarsen
Copy link
Member

Thanks a bunch!

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.

2 participants