search

pytorch / text

Models, data loaders and abstractions for language processing, powered by PyTorch
https://pytorch.org/text
BSD 3-Clause "New" or "Revised" License
3.49k stars 815 forks source link

Add file_lock when loading SentencePieceModel #2142

Closed joecummings closed 1 year ago

joecummings commented 1 year ago

Summary: SentencePieceModel loading can cause a RuntimeError when concurrent threads try to load/download it (e.g. when using T5 tokenizer in a DDP model training). Adding a file lock ensures the first thread to acquire the lock will actually download the model and the other ones will just use the existing path (which will not contain a partially downloaded model).

This diff was inspired by D42686913 and reverts D44566854 behavior (there is no need to overwrite anymore).

It should also disable unit test flakiness such as https://www.internalfb.com/intern/test/281475067136403?ref_report_id=0 and solve https://fb.workplace.com/groups/pytorchtext/permalink/920234369294862/.

Reviewed By: joecummings

Differential Revision: D44604474

fbshipit-source-id: 1c117fb6d1e72cce31cbf30bf72d513ad535b0d4

github-advanced-security[bot] commented 1 year ago

You have successfully added a new CodeQL configuration .github/workflows/codeql.yml:build. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.

joecummings commented 1 year ago

This seems to be a suitable stdlib fix: https://gist.github.com/jirihnidek/430d45c54311661b47fb45a3a7846537

mthrok commented 1 year ago

https://gist.github.com/jirihnidek/430d45c54311661b47fb45a3a7846537

IIRC fcntl-based approach is not portable to Windows, so need an alternative for that.

joecummings commented 1 year ago

Looking into using portalocker as a soln.