Remove hack for dynamic modules and use Python functions instead
Author: sgugger
Pull request: yes

# What does this PR do?

#21646 introduced a new way of dealing with dynamic modules locally to avoid a flaky failure in the CI, using a bit of hack with subprocess python commands. This seems to cause problem with distributed runs using this code (#22506 and was also reported internally as failing with big code experiment in the cluster).

It turns out there is a simple way to reset the cache which should be used when copying new modules, this PR uses that solution. I have run the scripts to reproduce the flakiness given in #21646 and didn't get any issue with the changes in this PR.

Fixes #22506

Comments:
_The documentation is not available anymore as the PR was closed or merged._

I confirm that this fixes a race condition discovered today with:

```
from transformers import GPT2Config, GPT2LMHeadModel, AutoModelForCausalLM, AutoConfig
model_name = "bigcode/santacoder"
config = AutoConfig.from_pretrained(model_name, trust_remote_code=True)
```

which fails randomly when multiple processes run it in parallel.

```
[default0]:Traceback (most recent call last):
[default0]:  File "<string>", line 1, in <module>
[default0]:FileNotFoundError: [Errno 2] No such file or directory: '/fsx/m4/modules/transformers_modules/bigcode/santacoder/bb63c0e145ad465df0a97dec285a949c9042523c/configuration_gpt2_mq.py'
```

the problem goes away with this PR.

I suspect a race condition is:

https://github.com/huggingface/transformers/blob/159ff3342c576ccf26cb00fb9510666ed626f42d/src/transformers/dynamic_module_utils.py#L173

which copies from multiple files to the same single target destination. This fails on FSX distributed filesystem with more than 2 dist processes.

Please note that the exception is not trapped and comes from the sub-process.

> It turns out there is a simple way to reset the cache which should be used when copying new modules, this PR uses that solution. I have run the scripts to reproduce the flakiness given in https://github.com/huggingface/transformers/pull/21646 and didn't get any issue with the changes in this PR.

I did notice that the SHA of the cached entry kept on changing. Why would one need to reset the cache, other than in special cases? Isn't the whole point of a cache is not to do anything but to load the file immediately? 

The problem we had was not on CI, so I'm not sure why the reset code was even run. Perhaps there is a need to check the reset isn't performed unless asked explicitly? Since it appears to be happening since we shouldn't have run into this issue in the first place if this code was meant to be run on reset only. Please correct me if I'm missing something.

> Why would one need to reset the cache, other than in special cases?

If you do not reset the cache after adding a new module, the import system of Python will not find it (see the doc of [`importlib.invalidate_caches()`](https://docs.python.org/3/library/importlib.html#importlib.invalidate_caches).

The function is only called when such a new module is added (new init, or newly copied dynamic code file) as you can see in the PR. It will only happen repeatedly if you keep downloading new models with dynamic code (or make it appear as such by doing save_pretrained then from_pretrained from different temp folders).

In any case, this situation will become even more rare in the near future when we will stop moving around those files with the code in each repo but trust only one source of truth.

But I'm not adding a new module, I'm rerunning the [same 1 line of code](https://github.com/huggingface/transformers/pull/22537#issuecomment-1495215758).

Is there something special about `"bigcode/santacoder"` that it never gets cached?

There should be 2 different behaviors:
1. first time - when it's downloaded 
2. 2nd and onward time when it's already cached.

no?

I am confused about what sha of the cached entry keeps changing. Could you elaborate? I added a print statement the five times `importlib.invalidate_caches()` is called after this PR and I can confirm it is never called once the model is cached when I run your sample above:
```py
from transformers import GPT2Config, GPT2LMHeadModel, AutoModelForCausalLM, AutoConfig
model_name = "bigcode/santacoder"
config = AutoConfig.from_pretrained(model_name, trust_remote_code=True)
```

I'm not exactly sure of the exact behavior, I was using several debug scripts and it seemed to be cycling between `bb63c0e145ad465df0a97dec285a949c9042523c` and `6a4fb77ff71c32c34dc8c61af500c7a7ca17c1a6`

But I wasn't talking about this, I was asking why was re-running this script with 4 processes:

```
from transformers import GPT2Config, GPT2LMHeadModel, AutoModelForCausalLM, AutoConfig
model_name = "bigcode/santacoder"
config = AutoConfig.from_pretrained(model_name, trust_remote_code=True)
```

kept re-rerunning the reset code - as it was failing most of the time in 1 or 2 out of 4 processes.

My thinking is that with the caching happened, even with the bug in resetting code, that resetting code shouldn't have been run.

e.g. here I reverted to transformers before this PR's fix:

1. run and make sure it's cached:

```
$ python test.py
Explicitly passing a `revision` is encouraged when loading a configuration with custom code to ensure no malicious code has been contributed in a newer revision.
```
it should be cached now for sure, right?

2. now it should just read the cached module

```
$ python -m torch.distributed.run --nproc_per_node=4 --nnodes=1 --tee 3 test.py 
[...]
[default0]:Explicitly passing a `revision` is encouraged when loading a configuration with custom code to ensure no malicious code has been contributed in a newer revision.
[default1]:Explicitly passing a `revision` is encouraged when loading a configuration with custom code to ensure no malicious code has been contributed in a newer revision.
[default3]:Explicitly passing a `revision` is encouraged when loading a configuration with custom code to ensure no malicious code has been contributed in a newer revision.
[default2]:Explicitly passing a `revision` is encouraged when loading a configuration with custom code to ensure no malicious code has been contributed in a newer revision.
[default1]:Traceback (most recent call last):
[default1]:  File "<string>", line 1, in <module>
[default1]:FileNotFoundError: [Errno 2] No such file or directory: '/fsx/m4/modules/transformers_modules/bigcode/santacoder/6a4fb77ff71c32c34dc8c61af500c7a7ca17c1a6/configuration_gpt2_mq.py'
[default0]:ModuleNotFoundError: No module named
[default0]:'transformers_modules.bigcode.santacoder.6a4fb77ff71c32c34dc8c61af500c7a7ca17c1a6.configuration_gpt2_mq'
ERROR:torch.distributed.elastic.multiprocessing.api:failed (exitcode: 1) local_rank: 0 (pid: 2722338) of binary: /fsx/m4/conda/stas-m4/bin/python
```
so that was my concern.

I am very confused, if you revert before this PR's fix, you will have the issue with the race condition. I don't get what the problem is after this PR?

there is no problem after this PR. It's OK, Sylvain.

If your question is why there was a change even with the file cached before this PR, it was because of a hack we implemented instead of using the proper way provided with `importlib.invalidate_cache()` (which we didn't know about), where the file with the code was deleted and recreated before each use. This is obviously bad for race conditions, hence the proper fix in this PR :-)

Aha! Thank you for clarifying the cause, Sylvain.

Thank you for removing the ugly hack I added 💯