Metric evaluation problems in multi-node, shared file system
Opened by gullabi (closed)

## Describe the bug
Metric evaluation fails in multi-node within a shared file system, because the master process cannot find the lock files from other nodes. (This issue was originally mentioned in the transformers repo https://github.com/huggingface/transformers/issues/17412)

## Steps to reproduce the bug

1. clone [this huggingface model](https://huggingface.co/PereLluis13/wav2vec2-xls-r-300m-ca-lm) and replace the `run_speech_recognition_ctc.py` script with the version in the gist [here](https://gist.github.com/gullabi/3f66094caa8db1c1e615dd35bd67ec71#file-run_speech_recognition_ctc-py).
2. Setup the `venv` according to the requirements of the model file plus `datasets==2.0.0`, `transformers==4.18.0` and `torch==1.9.0`
3. Launch the runner in a distributed environment which has a shared file system for two nodes, preferably with SLURM. Example [here](https://gist.github.com/gullabi/3f66094caa8db1c1e615dd35bd67ec71)

Specifically for the datasets, for the distributed setup the `load_metric` is called as:
```
    process_id=int(os.environ["RANK"])
    num_process=int(os.environ["WORLD_SIZE"])
    eval_metrics = {metric: load_metric(metric,
                                        process_id=process_id,
                                        num_process=num_process,
                                        experiment_id="slurm")
                    for metric in data_args.eval_metrics}
```

## Expected results
The training should not fail, due to the failure of the `Metric.compute()` step.

## Actual results
For the test I am executing the world size is 4, with 2 GPUs in 2 nodes. However the process is not finding the necessary lock files 
```
  File "/gpfs/projects/bsc88/speech/asr/wav2vec2-xls-r-300m-ca-lm/run_speech_recognition_ctc.py", line 841, in <module>
    main()
  File "/gpfs/projects/bsc88/speech/asr/wav2vec2-xls-r-300m-ca-lm/run_speech_recognition_ctc.py", line 792, in main
    train_result = trainer.train(resume_from_checkpoint=checkpoint)
  File "/gpfs/projects/bsc88/projects/speech-tech-resources/venv_amd_speech/lib/python3.7/site-packages/transformers/trainer.py", line 1497, in train
    self._maybe_log_save_evaluate(tr_loss, model, trial, epoch, ignore_keys_for_eval)
  File "/gpfs/projects/bsc88/projects/speech-tech-resources/venv_amd_speech/lib/python3.7/site-packages/transformers/trainer.py", line 1624, in _maybe_log_save_evaluate
    metrics = self.evaluate(ignore_keys=ignore_keys_for_eval)
  File "/gpfs/projects/bsc88/projects/speech-tech-resources/venv_amd_speech/lib/python3.7/site-packages/transformers/trainer.py", line 2291, in evaluate
    metric_key_prefix=metric_key_prefix,
  File "/gpfs/projects/bsc88/projects/speech-tech-resources/venv_amd_speech/lib/python3.7/site-packages/transformers/trainer.py", line 2535, in evaluation_loop
    metrics = self.compute_metrics(EvalPrediction(predictions=all_preds, label_ids=all_labels))
  File "/gpfs/projects/bsc88/speech/asr/wav2vec2-xls-r-300m-ca-lm/run_speech_recognition_ctc.py", line 742, in compute_metrics
    metrics = {k: v.compute(predictions=pred_str, references=label_str) for k, v in eval_metrics.items()}
  File "/gpfs/projects/bsc88/speech/asr/wav2vec2-xls-r-300m-ca-lm/run_speech_recognition_ctc.py", line 742, in <dictcomp>
    metrics = {k: v.compute(predictions=pred_str, references=label_str) for k, v in eval_metrics.items()}
  File "/gpfs/projects/bsc88/projects/speech-tech-resources/venv_amd_speech/lib/python3.7/site-packages/datasets/metric.py", line 419, in compute
    self.add_batch(**inputs)
  File "/gpfs/projects/bsc88/projects/speech-tech-resources/venv_amd_speech/lib/python3.7/site-packages/datasets/metric.py", line 465, in add_batch
    self._init_writer()
  File "/gpfs/projects/bsc88/projects/speech-tech-resources/venv_amd_speech/lib/python3.7/site-packages/datasets/metric.py", line 552, in _init_writer
    self._check_rendez_vous()  # wait for master to be ready and to let everyone go
  File "/gpfs/projects/bsc88/projects/speech-tech-resources/venv_amd_speech/lib/python3.7/site-packages/datasets/metric.py", line 342, in _check_rendez_vous
    ) from None
ValueError: Expected to find locked file /home/bsc88/bsc88474/.cache/huggingface/metrics/wer/default/slurm-4-0.arrow.lock from process 3 but it doesn't exist.
```

When I look at the cache directory, I can see all the lock files in principle:
```
/home/bsc88/bsc88474/.cache/huggingface/metrics/wer/default/slurm-4-0.arrow
/home/bsc88/bsc88474/.cache/huggingface/metrics/wer/default/slurm-4-0.arrow.lock
/home/bsc88/bsc88474/.cache/huggingface/metrics/wer/default/slurm-4-1.arrow
/home/bsc88/bsc88474/.cache/huggingface/metrics/wer/default/slurm-4-1.arrow.lock
/home/bsc88/bsc88474/.cache/huggingface/metrics/wer/default/slurm-4-2.arrow
/home/bsc88/bsc88474/.cache/huggingface/metrics/wer/default/slurm-4-2.arrow.lock
/home/bsc88/bsc88474/.cache/huggingface/metrics/wer/default/slurm-4-3.arrow
/home/bsc88/bsc88474/.cache/huggingface/metrics/wer/default/slurm-4-3.arrow.lock
/home/bsc88/bsc88474/.cache/huggingface/metrics/wer/default/slurm-4-rdv.lock
```

I see that there was another related issue here https://github.com/huggingface/datasets/issues/1942, but it seems to have resolved via https://github.com/huggingface/datasets/pull/1966. Let me know if there is problem with how I am calling the `load_metric` or whether I need to make changes to the `.compute()` steps.

## Environment info
- `datasets` version: 2.0.0
- Platform: Linux-4.18.0-147.8.1.el8_1.x86_64-x86_64-with-centos-8.1.1911-Core
- Python version: 3.7.4
- PyArrow version: 7.0.0
- Pandas version: 1.3.0



Comments:
If you call `metric.compute` in a distributed setup like yours, then `metric.compute` is called in each process. `metric.compute` first calls `metric.add_batch`, and it looks like your error appears at that stage.

To make sure that all the processes have started writing their predictions/references at the same time, each process waits for process 0 to lock `slurm-{world_size}-0.arrow.lock`. Process 0 locks this file when `metric.add_batch` is called, so here when `metric.compute` is called.

Therefore your error can happen when process 0 takes too much time to call `metric.compute` compared to process 3 (>100 seconds by default). I haven't tried running your code but could it be the case ?

I guess it could also happen if you run multiple times the same distributed job at the same time with the same `experiment_id` because they would collide.


We've finally been able to isolate the problem, it wasn't a timing problem, but rather a file locking one. 
The locks produced by calling `flock` where not visible between nodes (so the master node couldn't check other node's locks nor the other way around). 

We are now having issues with the pre-processing in our runner script, but are not related with the rendezvous process during the evaluation phase. We will let you know about it once we address it. 

Our solution to the rendezvous is as follows:
- We solved the problem by calling `lockf` instead of `flock`.
- We had to change slightly the `_check_all_processes_locks` method so that the main process (i.e. process 0) didn't check it's own lock (because `lockf` permits recursive locks and thus checking it only replaced the current lock with a new one). 

We use a shared file system between nodes using GPFS in our cluster setup. Maybe the difference between the behavior we see with respect to your usage in multi-node executions comes from that fact. Which file system scheme do you use for the multi-node executions? 

`lockf` seems to work in more settings than `flock`, so maybe we could write a PR so you could test it in your environment. 

Cool, I'm glad you managed to make evaluation work :)

I'm not completely aware of the differences between lockf and flock, but I've read somewhere that flock is preferable over lockf in multithreading and multiprocessing situations. Here we definitely are in such a situation so unless it is super important I don't think we will switch to lockf

> * We had to change slightly the `_check_all_processes_locks` method so that the main process (i.e. process 0) didn't check it's own lock (because `lockf` permits recursive locks and thus checking it only replaced the current lock with a new one).

Hi @panserbjorn , Can you share your `_check_all_processes_locks` function? thanks!

```
def _check_all_processes_locks(self):
        expected_lock_file_names = [
            os.path.join(self.data_dir, f"{self.experiment_id}-{self.num_process}-{process_id}.arrow.lock")
            for process_id in range(self.num_process)
        ]
        #for expected_lock_file_name in expected_lock_file_names: # OUR CHANGE process 0 shouldn't check its own lock
        for expected_lock_file_name in expected_lock_file_names[1:]:
            nofilelock = FileFreeLock(expected_lock_file_name)
            try:
                nofilelock.acquire(timeout=self.timeout)
            except Timeout:
                raise ValueError(
                    f"Expected to find locked file {expected_lock_file_name} from process {self.process_id} but it doesn't exist."
                )
            else:
                nofilelock.release()
```

### Changed files:
- metric.py file in the datasets library  
- filelock.py file in the datasets/utils library. 


Changes we made:

1. We changed the flock for lockf 
   flock and lockf both perform a lock over a file (like the lock for writing). 
   The difference is that flock only works in local file systems, but if you have a shared file system (like what we have in the clusters) the flock fails to “see” the lock of another node. The only disadvantage we had was that a single process couldn’t detect it’s own lock so we did the second change.
2. We prevented the process 0 (which is the one that coordinates the rendezvous) from checking its own lock on its arrow because it didn't work with lockf (as stated in the previous change). 
3. We made a second rendezvous so that all the process had the results of the metrics (other than the loss) and not only the process 0.
   What happened was that only process 0 computed the metric and that didn’t present any problem if you are using the loss. However, if you are using another metric, the only process which had the information to choose the best checkpoint at evaluation time was the process 0. But since the evaluation was performed over all processes, every process except the process 0 chose a bad check point (bad meaning it wasn’t the best one) because they didn’t have the information of the metric of the best checkpoint. 
   The consequence was that the evaluation was different from what would result if using only the best checkpoint, because each process chose a different checkpoint to run the evaluation and thus the numbers were often worse than the numbers that would be obtained if all processes choose the best checkpoint (correct one) to perform the evaluation of their samples. 
   We performed a second rendezvous so that all processes had the same best_metric and best_model as process 0 after the evaluation cycle. 


Metrics are deprecated in `datasets` and `evaluate` should be used instead: https://github.com/huggingface/evaluate