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

fix(worker): Adds before_update hook #18

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

delucca
Copy link
Contributor

@delucca delucca commented Apr 19, 2023

Hi!

I found this repository on spacy#8093 and I was trying to train a large model in multiple GPUs.

I've tried using the latest Spacy version (3.5) with spacy-transformers and it seems this repo doesn't support it since the training.before_update key in the config is quite new (and required).

So, I've added it and I was able to launch a new training session with that change.

@delucca
Copy link
Contributor Author

delucca commented Apr 19, 2023

Just a note, I'm still trying to run it. For some reason spacy-ray still only allocates memory on a single GPU. But, either way, that fix is required (and quite easy) in any case

@rmitsch
Copy link

rmitsch commented May 15, 2023

Hi @delucca, thanks for your contribution!

FYI, spacy-ray hasn't been updated recently and it was never quite ready for production use even for spacy v3.0. There are still a number of major limitations and bugs, see #14 (comment).

rmitsch
rmitsch previously approved these changes May 15, 2023
@rmitsch rmitsch dismissed their stale review May 15, 2023 09:09

Failing tests

Copy link
Member

@svlandeg svlandeg left a comment

Choose a reason for hiding this comment

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

There's other things we need to fix, including the CI tests, but this PR by itself makes sense so I'll merge that in. Thanks!

@svlandeg svlandeg merged commit 09ffba5 into explosion:master Jul 31, 2023
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.

3 participants