Allow passing model_args to ST

[satyamk7054]

Summary

Allow passing model_args to ST

Details

This fixes #2579.
New models like e5-mistral-7b-instruct use FP16 as the dtype.
However, when loaded using sentence_transformers, they are loaded with FP32. This is because
HF transformers uses FP32 as the default unless torch_dtype='auto' is passed, as mentioned here.

Passing "auto" to model_args does not work because of the below error. Moreover, the SentenceTransformers class does not currently expose a model_args param.

cls = <class 'transformers.models.bert.modeling_bert.BertModel'>, dtype = 'auto'

    @classmethod
    def _set_default_torch_dtype(cls, dtype: torch.dtype) -> torch.dtype:
        """
        Change the default dtype and return the previous one. This is needed when wanting to instantiate the model
        under specific dtype.
    
        Args:
            dtype (`torch.dtype`):
                a floating dtype to set to.
    
        Returns:
            `torch.dtype`: the original `dtype` that can be used to restore `torch.set_default_dtype(dtype)` if it was
            modified. If it wasn't, returns `None`.
    
        Note `set_default_dtype` currently only works with floating-point types and asserts if for example,
        `torch.int64` is passed. So if a non-float `dtype` is passed this functions will throw an exception.
        """
>       if not dtype.is_floating_point:
E       AttributeError: 'str' object has no attribute 'is_floating_point'

../venv/lib/python3.10/site-packages/transformers/modeling_utils.py:1412: AttributeError

Testing Done

Added a new unit tests that validates the dtype of the loaded model using the embedding tensor
created with it.

[tomaarsen]

As mentioned in #2579, I think this PR will be preferable to #2426 due the hacky nature of the latter. I do think that some slight changes in the API would be beneficial, however.

Thanks for taking the time to make your issue & PR, and for dealing with my slow responses.

Also, please do share your opinion on these suggestions.

[satyamk7054]

@tomaarsen could you please take a look at the updated PR?

[tomaarsen]

Two small nitpicks, but otherwise this is looking strong!

[satyamk7054]

@tomaarsen could you please look at this PR again?

[tomaarsen]

I will do some more local tests before I merge this, but I imagine that it's ready to go. I'll merge this into master and then propagate those changes into v3.0-pre-release.
For context, v3 should release quite soon (~2 weeks?). I am working on refactoring the sbert.net documentation, but it'll be released once that's done.

• Tom Aarsen

[tomaarsen]

Hello!

I've had to reintroduce the config. Unless we can fully remove it, we do need some of the model kwargs to be passed to the AutoConfig. The approach from your PR still required a config to be created due to the ifinstance(config, ...). For example, loading this model for testing would have unexpected behaviour because trust_remote_code was not passed to the AutoConfig. (similar issues with token, revision, etc.)

Instead, I've created an allow-list of keys that should be passed to the AutoConfig based on its documentation.

Secondarily, #2630 has requested simpler support to update certain settings from the tokenizer, so I'm having to reconsider the **model_kwargs approach.

• Tom Aarsen

[tomaarsen]

Hello @satyamk7054

I've done some more thinking, and it seems that there is some (albeit rather niche) needs to be able to specify parameters for the tokenizer and config (my comment on that), so I think it might be best to go with your original idea of letting users specify a model_args dictionary (but then called model_kwargs, that's a more accurate name I think) and passing it directly to the transformer. Idem with tokenizers and configs. What do you think?

• Tom Aarsen

[satyamk7054]

@tomaarsen Thank you for more testing and updating the PR! I read through the discussion in the linked issue and I think adding these additional kwargs makes sense (thank you for letting me know).

[tomaarsen]

@muellerzr @osanseviero I was wondering if you could have another quick look. I've changed my mind after a user required use_fast=False in their Tokenizer and another user wanted to load a model with a higher dropout for training (i.e., this can be passed to the config). We now have three ..._kwargs parameters to allow the users perfect control over the underlying transformers objects, while allowing me to introduce new parameters as well. I think this is much safer, as much as I dislike having users need to provide dictionaries with values.

This is still a fairly major change, so I wanted your opinions on this, if you have a bit of time.

• Tom Aarsen

[muellerzr]

Very nice! I like that much more, and am thankful you document where to find those easily! Keeps your side a bit easier too as things change/etc 🤗

[ZhengHongming888]

This PR is good that giving more flexible control over the transformers from 3 .. _kwargs because later on the user will need more /more freedom to add something on top of transformers. Just carefully take a look at this PR this morning .. :-) Good work! @satyamk7054 @tomaarsen