Skip to content

Fixed bugs #857 #860 #861 and #863#864

Open
andrealorenzani wants to merge 1 commit into
neonbjb:mainfrom
andrealorenzani:bugs/dependency_errors
Open

Fixed bugs #857 #860 #861 and #863#864
andrealorenzani wants to merge 1 commit into
neonbjb:mainfrom
andrealorenzani:bugs/dependency_errors

Conversation

@andrealorenzani

Copy link
Copy Markdown

No description provided.

@andrealorenzani

Copy link
Copy Markdown
Author

@andrealorenzani fixed bugs #857 #860 #861 and #863

@libTorrentUser

Copy link
Copy Markdown

Nice job, but I believe you should really consider downgrading transformers to 4.51.0 because, after that version, they changed some defaults and they are making the code 100% slower. Literally. Inferences that we taking 1 minute on 4.51.0 now take 2 minutes on 4.52.x or 4.53.x

There are some open issues on their github about this particular problem.

@andrealorenzani

Copy link
Copy Markdown
Author

I am not an expert of transformers and I just fixed the issues because I saw that a lot of people had the same blockers. I could downgrade, I am just a bit conscious that at the moment there are newer versions, up to 4.56.1 and I would go up instead of down.

If someone merge the PR I am happy to then make another one or leave it to someone more experienced to do it. Those libs are working together, retesting everything is out of scope for me

@libTorrentUser

Copy link
Copy Markdown

I agree that going up sounds like the better idea but, as far as I understood (I could be wrong here), they are not changing the defaults back and, since Tortoise use the defaults, the huge performance hit will remain.

As far as I know (only tested up to v4.53.xx) there should be no code changes at all. You already did the hard part. Just set the transformers version back to v.4.51.0 and compare the speed with the one you decided to use. If there is no difference you can stick with the newest, but if it is twice as slow, then you could either downgrade to v4.51.0 or set the value of the problematic argument (I think it is beam_size) back to the old default.

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