Make parallelism optional #311
Conversation
|
Oof, that error message has me stumped. I do like this solution though! I hope we can get it working. |
n1t0
force-pushed
the
optional-parallelism
branch
from
bbd4ce7 to
5f760df
Compare
| #[cfg(target_os = "linux")] | ||
| unsafe { | ||
| if !REGISTERED_FORK_CALLBACK { | ||
| libc::pthread_atfork(None, None, Some(child_after_fork)); |
There was a problem hiding this comment.
I didn't know about it either, and it seems to be working really well. This should let us have a very flawless experience. 🎉
Co-authored-by: Evan Pete Walsh <[email protected]>
|
Ok, everything should be good now. So, the behavior in Python is now the following:
Except for the I think we might wait for cuviper/rayon-cond#2 to be merged before we merge this, but this is now ready for review! |
|
Nice catch! I think it should work fine now. |
|
I just confirmed it works! 💯 |
|
Ok, let's merge this to include it in the upcoming release. We'll see what to do with the |
- Added TOKENIZERS_PARALLELISM=false in the envrc file. For more information you can visit huggingface/tokenizers#311 - orchestration/task_adapter.py - Added model checkpointing callbacks to the trainer. Saves the best model depending on the metric being tracked - models/task_adapter.py - Removed logging loss for every step in the test. It is enough to log at the end of test epoch - scripts/train_task/task-adapter.sh passes the options for number of workers to use in the dataloader; the metric to track among others
Related to #187
An attempt at implementing your first idea @epwalsh. I just didn't manage to have
map_withworking withmaybe_par_bridgebut everything else seems to be good!Let me know what you think! And if you have an idea for the
map_with, that'd be awesome, I couldn't make sense of the error...