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

[feat + fix] Add normalize_embeddings support to multi-process encoding; fix multi-process encoding on CUDA devices #2377

Merged

Conversation

tomaarsen
Copy link
Collaborator

@tomaarsen tomaarsen commented Dec 13, 2023

Supersedes #2252

Hello!

Pull Request overview

  • Add normalize_embeddings support to multi-process encoding
  • Fix multi-process encoding on CUDA-enabled devices
    • In particular, it caused all model weights to become 0.0
  • Add tests

Details

The normalize_embeddings functionality is fairly straightforward - it speaks for itself.

The encoding fix not quite so much. When the model is loaded on CUDA and start_multi_process_pool is called, then the model weights become 0.0 as soon as one of the processes start. There's a tad more info here. This is counteracted when the model is on CPU originally, so I move the model to cpu just prior to starting the encoding. I also use share_memory as it's recommended.

Thanks @TeisNP for starting this work!

  • Tom Aarsen

@tomaarsen tomaarsen merged commit 8af4744 into UKPLab:master Dec 13, 2023
8 checks passed
@tomaarsen tomaarsen deleted the feat/multi-process-encode-normalize branch December 13, 2023 15:31
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