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

Question : why is model_original_state necessary? #199

Closed
Yongtae723 opened this issue Nov 22, 2022 · 2 comments · Fixed by #201
Closed

Question : why is model_original_state necessary? #199

Yongtae723 opened this issue Nov 22, 2022 · 2 comments · Fixed by #201

Comments

@Yongtae723
Copy link
Contributor

Yongtae723 commented Nov 22, 2022

In SetFitModel, self.model_original_state is defined, as you can see here.

But I don't see this variable being used again. So I wonder why you define that.
Since self.model_original_state contains the weights of sentence transformers, which leads to a heavy model class, so if it is not necessary, we can delete it to make a light model.

Could you tell me your opinion?

Thank you.
I am looking forward to your reply.

Yongtae

@tomaarsen
Copy link
Member

Hello!

A model_original_state variable is used a few times in the different scripts, so perhaps it's a relic from converting the scripts into a larger repository?

I agree that it does not seem needed. I'll make a PR so the maintainers can easily accept this change, should they agree with us.

  • Tom Aarsen

@Yongtae723
Copy link
Contributor Author

@tomaarsen
Hi Tom! I confirmed your PR!

Thank you for it!

Yongtae

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 a pull request may close this issue.

2 participants