Repository: huggingface/transformers (language: python)

Issue:
Breaking change due to `multiprocessing.Process` when loading `pytorch_model.bin`-based model 
## Bug Overview
* Loading any `transformers` models fails if:
  * the model only has a `pytorch_model.bin`, and
  * you're not inside of `__name__ == "__main__"`

(Taken from https://github.com/huggingface/transformers/pull/34966#issuecomment-2538598145)

## Details
In short: `multiprocessing.Process` never works when not inside of `__name__ == "__main__"`. I recognize that most programs should be using that line, but I'd rather not force it on my users.

If one of my users loads any model that only has a `pytorch_model.bin`, then it'll fail, e.g.:
```python
from sentence_transformers import SentenceTransformer

model = SentenceTransformer("embaas/sentence-transformers-gte-base")
```
or
```python
from sentence_transformers import CrossEncoder

model = CrossEncoder("cross-encoder/ms-marco-MiniLM-L-6-v2")
```
which internally call
```python
from transformers import AutoModel

model = AutoModel.from_pretrained("embaas/sentence-transformers-gte-base")
```
or
```python
from transformers import AutoModelForSequenceClassification

model = AutoModelForSequenceClassification.from_pretrained("cross-encoder/ms-marco-MiniLM-L-6-v2")
```

All of these get:
```
RuntimeError:
        An attempt has been made to start a new process before the
        current process has finished its bootstrapping phase.

        This probably means that you are not using fork to start your
        child processes and you have forgotten to use the proper idiom
        in the main module:

            if __name__ == '__main__':
                freeze_support()
                ...

        The "freeze_support()" line can be omitted if the program
        is not going to be frozen to produce an executable.

        To fix this issue, refer to the "Safe importing of main module"
        section in https://docs.python.org/3/library/multiprocessing.html
```

Edit: To prevent people experiencing errors, I've updated all [`cross-encoder` models](https://huggingface.co/cross-encoder) to safetensors. So you can't reproduce those anymore without specifying the `revision`.

- Tom Aarsen
Fix flaky test execution caused by `Thread`
# What does this PR do?

As discussed offline:

> Would it possible if we change Thread to Process below :pray: ?
I know it's not good to change code that is working and making sense for the sole purpose of testing, but Thread  (without join) here making some test(s) flaky failing
test_cached_model_has_minimum_calls_to_head
where we count the number of calls to Hub. Some other tests that (eventually) calling from_pretrained  entering Thread line  will affect test_cached_model_has_minimum_calls_to_head
because they are running at the same time in the same process.
There are other solutions, but the changes involves patching (a lot of) tests (so at conftest level which is not good neither IMO.
https://github.com/huggingface/transformers/blob/d5cf91b3462b5ed57260074a0708f09b16e787a8/src/transformers/modeling_utils.py#L3842

----------------------------------------------

To check the PR works (make sure tensorflow is available): 

> python gradio2.py

where `script.py`
```python
for i in range(20):
    import os
    os.system('python -m pytest -v tests/models/auto/test_modeling_auto.py tests/models/auto/test_modeling_tf_auto.py -k "test_from_pretrained_identifier or test_cached_model_has_minimum_calls_to_head"')
    # or running several times on CI workflow: it's flaky and not easy to reproduce
    # or add `time.sleep(0.5)` at the beginning of `auto_convenanorsion`

```
Fix flaky test execution caused by `Thread`
# What does this PR do?

As discussed offline:

> Would it possible if we change Thread to Process below :pray: ?
I know it's not good to change code that is working and making sense for the sole purpose of testing, but Thread  (without join) here making some test(s) flaky failing
test_cached_model_has_minimum_calls_to_head
where we count the number of calls to Hub. Some other tests that (eventually) calling from_pretrained  entering Thread line  will affect test_cached_model_has_minimum_calls_to_head
because they are running at the same time in the same process.
There are other solutions, but the changes involves patching (a lot of) tests (so at conftest level which is not good neither IMO.
https://github.com/huggingface/transformers/blob/d5cf91b3462b5ed57260074a0708f09b16e787a8/src/transformers/modeling_utils.py#L3842

----------------------------------------------

To check the PR works (make sure tensorflow is available): 

> python gradio2.py

where `script.py`
```python
for i in range(20):
    import os
    os.system('python -m pytest -v tests/models/auto/test_modeling_auto.py tests/models/auto/test_modeling_tf_auto.py -k "test_from_pretrained_identifier or test_cached_model_has_minimum_calls_to_head"')
    # or running several times on CI workflow: it's flaky and not easy to reproduce
    # or add `time.sleep(0.5)` at the beginning of `auto_convenanorsion`

```


Discussion / hints:
On it :-) Thanks for reporting
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_34966). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Hi @ydshieh - FYI this change appears to have broken some unit tests in DeepSpeed, specifically where we download a model.  I'll look at ways to resolve this, I assume it is an issue in multiprocessing/threading.
@loadams Thank you for informing us. Hope there is a way that could work in both cases 🙏 Let me know if there is anything we can help here.
@ydshieh I'm also experiencing issues here for Sentence Transformers/Cross Encoder models, see https://github.com/UKPLab/sentence-transformers/issues/3129

In short: `multiprocessing.Process` never works when not inside of `__name__ == "__main__"`. I recognize that most programs should be using that line, but I'd rather not force it on my users.

If one of my users loads any model that only has a `pytorch_model.bin`, then it'll fail, e.g.:
```python
from sentence_transformers import SentenceTransformer

model = SentenceTransformer("embaas/sentence-transformers-gte-base")
```
or
```python
from sentence_transformers import CrossEncoder

model = CrossEncoder("cross-encoder/ms-marco-MiniLM-L-6-v2")
```
which internally call
```python
from transformers import AutoModel

model = AutoModel.from_pretrained("embaas/sentence-transformers-gte-base")
```
or
```python
from transformers import AutoModelForSequenceClassification

model = AutoModelForSequenceClassification.from_pretrained("cross-encoder/ms-marco-MiniLM-L-6-v2")
```

All of these get:
```
RuntimeError:
        An attempt has been made to start a new process before the
        current process has finished its bootstrapping phase.

        This probably means that you are not using fork to start your
        child processes and you have forgotten to use the proper idiom
        in the main module:

            if __name__ == '__main__':
                freeze_support()
                ...

        The "freeze_support()" line can be omitted if the program
        is not going to be frozen to produce an executable.

        To fix this issue, refer to the "Safe importing of main module"
        section in https://docs.python.org/3/library/multiprocessing.html
```

- Tom Aarsen
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_34966). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Hi @ydshieh - FYI this change appears to have broken some unit tests in DeepSpeed, specifically where we download a model.  I'll look at ways to resolve this, I assume it is an issue in multiprocessing/threading.
@loadams Thank you for informing us. Hope there is a way that could work in both cases 🙏 Let me know if there is anything we can help here.
@ydshieh I'm also experiencing issues here for Sentence Transformers/Cross Encoder models, see https://github.com/UKPLab/sentence-transformers/issues/3129

In short: `multiprocessing.Process` never works when not inside of `__name__ == "__main__"`. I recognize that most programs should be using that line, but I'd rather not force it on my users.

If one of my users loads any model that only has a `pytorch_model.bin`, then it'll fail, e.g.:
```python
from sentence_transformers import SentenceTransformer

model = SentenceTransformer("embaas/sentence-transformers-gte-base")
```
or
```python
from sentence_transformers import CrossEncoder

model = CrossEncoder("cross-encoder/ms-marco-MiniLM-L-6-v2")
```
which internally call
```python
from transformers import AutoModel

model = AutoModel.from_pretrained("embaas/sentence-transformers-gte-base")
```
or
```python
from transformers import AutoModelForSequenceClassification

model = AutoModelForSequenceClassification.from_pretrained("cross-encoder/ms-marco-MiniLM-L-6-v2")
```

All of these get:
```
RuntimeError:
        An attempt has been made to start a new process before the
        current process has finished its bootstrapping phase.

        This probably means that you are not using fork to start your
        child processes and you have forgotten to use the proper idiom
        in the main module:

            if __name__ == '__main__':
                freeze_support()
                ...

        The "freeze_support()" line can be omitted if the program
        is not going to be frozen to produce an executable.

        To fix this issue, refer to the "Safe importing of main module"
        section in https://docs.python.org/3/library/multiprocessing.html
```

- Tom Aarsen

Fix patch:
diff --git a/src/transformers/modeling_utils.py b/src/transformers/modeling_utils.py
index f349847b1fd7a1..c86559e62f94ea 100755
--- a/src/transformers/modeling_utils.py
+++ b/src/transformers/modeling_utils.py
@@ -29,7 +29,7 @@
 from contextlib import contextmanager
 from dataclasses import dataclass
 from functools import partial, wraps
-from multiprocessing import Process
+from threading import Thread
 from typing import Any, Callable, Dict, List, Optional, Set, Tuple, Type, TypeVar, Union
 from zipfile import is_zipfile
 
@@ -3825,11 +3825,11 @@ def from_pretrained(
                                     **has_file_kwargs,
                                 }
                                 if not has_file(pretrained_model_name_or_path, safe_weights_name, **has_file_kwargs):
-                                    Process(
+                                    Thread(
                                         target=auto_conversion,
                                         args=(pretrained_model_name_or_path,),
                                         kwargs={"ignore_errors_during_conversion": True, **cached_file_kwargs},
-                                        name="Process-auto_conversion",
+                                        name="Thread-auto_conversion",
                                     ).start()
                         else:
                             # Otherwise, no PyTorch file was found, maybe there is a TF or Flax model file.
diff --git a/src/transformers/safetensors_conversion.py b/src/transformers/safetensors_conversion.py
index 5c0179350ea2ef..f1612d3ea57c98 100644
--- a/src/transformers/safetensors_conversion.py
+++ b/src/transformers/safetensors_conversion.py
@@ -67,7 +67,7 @@ def get_conversion_pr_reference(api: HfApi, model_id: str, **kwargs):
     # security breaches.
     pr = previous_pr(api, model_id, pr_title, token=token)
 
-    if pr is None or (not private and pr.author != "SFConvertBot"):
+    if pr is None or (not private and pr.author != "SFconvertbot"):
         spawn_conversion(token, private, model_id)
         pr = previous_pr(api, model_id, pr_title, token=token)
     else:


Test patch:
diff --git a/src/transformers/testing_utils.py b/src/transformers/testing_utils.py
index 30f7b5a68fb2c0..409f274d41eb17 100644
--- a/src/transformers/testing_utils.py
+++ b/src/transformers/testing_utils.py
@@ -28,6 +28,7 @@
 import subprocess
 import sys
 import tempfile
+import threading
 import time
 import unittest
 from collections import defaultdict
@@ -2311,12 +2312,28 @@ class RequestCounter:
 
     def __enter__(self):
         self._counter = defaultdict(int)
-        self.patcher = patch.object(urllib3.connectionpool.log, "debug", wraps=urllib3.connectionpool.log.debug)
+        self._thread_id = threading.get_ident()
+        self._extra_info = []
+
+        def patched_with_thread_info(func):
+            def wrap(*args, **kwargs):
+                self._extra_info.append(threading.get_ident())
+                return func(*args, **kwargs)
+
+            return wrap
+
+        self.patcher = patch.object(
+            urllib3.connectionpool.log, "debug", side_effect=patched_with_thread_info(urllib3.connectionpool.log.debug)
+        )
         self.mock = self.patcher.start()
         return self
 
     def __exit__(self, *args, **kwargs) -> None:
-        for call in self.mock.call_args_list:
+        assert len(self.mock.call_args_list) == len(self._extra_info)
+
+        for thread_id, call in zip(self._extra_info, self.mock.call_args_list):
+            if thread_id != self._thread_id:
+                continue
             log = call.args[0] % call.args[1:]
             for method in ("HEAD", "GET", "POST", "PUT", "DELETE", "CONNECT", "OPTIONS", "TRACE", "PATCH"):
                 if method in log: