Revert changes to multi-GPU evaluator calls#3463
Conversation
|
happy to discuss to find better solutions! |
|
This is the crux of the error at the moment: In short, each individual device seems to check for the best metric deep in transformers for some reason (I'd think that only the first device has to do that), so it's incompatible with The best solution is full multi-GPU evaluation support, and the second best is merging this PR so that all GPUs evaluate again (even if that's mostly a waste of GPU time). I'll merge this now to avoid the
|
There was a problem hiding this comment.
Pull Request Overview
This PR reverts changes to multi-GPU evaluator calls that were introduced in PR #3442, which reportedly caused crashes during experiments. The main purpose is to restore the previous behavior by re-adding a logging suppression mechanism for non-primary processes during evaluation.
Key changes:
- Adds back the
disable_loggingcontext manager utility function - Restores conditional logging suppression for non-primary processes during evaluation
- Reverts the evaluator execution condition to run on all processes instead of only the primary process
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| sentence_transformers/util/misc.py | Adds the disable_logging context manager function |
| sentence_transformers/util/init.py | Exports the disable_logging function |
| sentence_transformers/trainer.py | Reverts evaluator execution logic and adds logging suppression for non-primary processes |
Hello!
Pull Request overview
Details
Reverts part of #3442. According to experiments by @arthurbr11, this introduced a crash. I haven't had a chance to test it myself yet, will do so later.
cc @NohTow