Fix for multi gpu setup training with a single GPU. #974
Conversation
check_nvidia() originally spawns a new process for nvidia-smi, thus bypassing that GPU count might be limited by an OS environmental variable as this won't be reflected in the new process. Added check for if GPU is limited by OS environ, if multiple, raises error like original behaviour. If only one GPU enabled, only returns output for that GPU.
Add fixed code to the trainer patcher.
Fixed variable misname in trainer patcher.
There was a problem hiding this comment.
There are several instances where potential failures (like in convert_to_fast_tokenizer, try_fix_tokenizer, and assert_same_tokenization) are not properly logged or handled. Instead of silently returning the slow tokenizer, there should be logs or warnings for failed conversions or mismatches.
if not check_vocab or not check_special:
logger.warning("Vocab or special tokens do not match between slow and fast tokenizer.")
return slow_tokenizer
|
@RahulVadisetty91 Hello, your review does not seem relevant to my PR. Can you elaborate on the relevancy? |
| index_for_cuda = -1 | ||
| if "CUDA_VISIBLE_DEVICES" in os.environ: | ||
| index_for_cuda = os.environ["CUDA_VISIBLE_DEVICES"] |
There was a problem hiding this comment.
| index_for_cuda = -1 | |
| if "CUDA_VISIBLE_DEVICES" in os.environ: | |
| index_for_cuda = os.environ["CUDA_VISIBLE_DEVICES"] | |
| index_for_cuda = os.environ.get("CUDA_VISIBLE_DEVICES", -1) |
There was a problem hiding this comment.
The next few lines would take care of that I suppose.
|
@Sehyo @hife-ai isn't it better till the dev is ongoing someone updates this in ReadMe about using os.environ specifically this is a problem for the notebooks also I think we should use a device argument which can be a list or int or leverage that everywhere rather than checking like this. And if someone specifies more devices then the code can exit in the start itself saying multi GPU is not supported. What you say ? |
|
@giuliabaldini in that case can you please link that here? |
|
@Datta0 I am still testing that it works, I will open the PR as soon as I am sure and link it here! |
I am not sure why you would think that is the case. The trainer patcher clearly contains code that will break multi gpu functionality without my PR. This is the appropriate solution. |
|
Sure I could do it tomorrow if necessary |
|
Let me know if you can't and I will do it! |
check_nvidia() originally spawns a new process for nvidia-smi, thus bypassing that GPU count might be limited by an OS environmental variable as this won't be reflected in the new process.
Added check for if GPU is limited by OS environ, if multiple, raises error like original behaviour.
If only one GPU enabled, only returns output for that GPU.