Title: Possible cache miss in datasets
Author: ofirzaf (NONE)
State: closed
Created: 2021-01-11T15:37:31Z

Hi,

I am using the datasets package and even though I run the same data processing functions, datasets always recomputes the function instead of using cache.
I have attached an example script that for me reproduces the problem.
In the attached example the second map function always recomputes instead of loading from cache.
Is this a bug or am I doing something wrong?
Is there a way for fix this and avoid all the recomputation?

Thanks

Edit:
transformers==3.5.1
datasets==1.2.0

```
from datasets import load_dataset
from transformers import AutoTokenizer

datasets = load_dataset('wikitext', 'wikitext-103-raw-v1')
tokenizer = AutoTokenizer.from_pretrained('bert-base-uncased', use_fast=True)


column_names = datasets["train"].column_names
text_column_name = "text" if "text" in column_names else column_names[0]
def tokenize_function(examples):
    return tokenizer(examples[text_column_name], return_special_tokens_mask=True)

tokenized_datasets = datasets.map(
    tokenize_function,
    batched=True,
    num_proc=60,
    remove_columns=[text_column_name],
    load_from_cache_file=True,
)
max_seq_length = tokenizer.model_max_length
def group_texts(examples):
    # Concatenate all texts.
    concatenated_examples = {
        k: sum(examples[k], []) for k in examples.keys()}
    total_length = len(concatenated_examples[list(examples.keys())[0]])
    # We drop the small remainder, we could add padding if the model supported it instead of this drop, you can
    # customize this part to your needs.
    total_length = (total_length // max_seq_length) * max_seq_length
    # Split by chunks of max_len.
    result = {
        k: [t[i: i + max_seq_length]
            for i in range(0, total_length, max_seq_length)]
        for k, t in concatenated_examples.items()
    }
    return result

tokenized_datasets = tokenized_datasets.map(
    group_texts,
    batched=True,
    num_proc=60,
    load_from_cache_file=True,
)
print(tokenized_datasets)

print('finished')
```

Comments:
Thanks for reporting !
I was able to reproduce thanks to your code and find the origin of the bug.
The cache was not reusing the same file because one object was not deterministic. It comes from a conversion from `set` to `list` in the `datasets.arrrow_dataset.transmit_format` function, where the resulting list would not always be in the same order and therefore the function that computes the hash used by the cache would not always return the same result.
I'm opening a PR to fix this.

Also we plan to do a new release in the coming days so you can expect the fix to be available soon.
Note that you can still specify `cache_file_name=` in the second `map()` call to name the cache file yourself if you want to.

Thanks for the fast reply, waiting for the fix :)

I tried to use `cache_file_names` and wasn't sure how, I tried to give it the following:
```
tokenized_datasets = tokenized_datasets.map(
    group_texts,
    batched=True,
    num_proc=60,
    load_from_cache_file=True,
    cache_file_names={k: f'.cache/{str(k)}' for k in tokenized_datasets}
)
```

and got an error:
```
multiprocess.pool.RemoteTraceback:
"""
Traceback (most recent call last):
  File "/venv/lib/python3.6/site-packages/multiprocess/pool.py", line 119, in worker
    result = (True, func(*args, **kwds))
  File "/venv/lib/python3.6/site-packages/datasets/arrow_dataset.py", line 157, in wrapper
    out: Union["Dataset", "DatasetDict"] = func(self, *args, **kwargs)
  File "/venv/lib/python3.6/site-packages/datasets/fingerprint.py", line 163, in wrapper
    out = func(self, *args, **kwargs)
  File "/venv/lib/python3.6/site-packages/datasets/arrow_dataset.py", line 1491, in _map_single
    tmp_file = tempfile.NamedTemporaryFile("wb", dir=os.path.dirname(cache_file_name), delete=False)
  File "/usr/lib/python3.6/tempfile.py", line 690, in NamedTemporaryFile
    (fd, name) = _mkstemp_inner(dir, prefix, suffix, flags, output_type)
  File "/usr/lib/python3.6/tempfile.py", line 401, in _mkstemp_inner
    fd = _os.open(file, flags, 0o600)
FileNotFoundError: [Errno 2] No such file or directory: '_00000_of_00060.cache/tmpsvszxtop'
"""

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "test.py", line 48, in <module>
    cache_file_names={k: f'.cache/{str(k)}' for k in tokenized_datasets}
  File "/venv/lib/python3.6/site-packages/datasets/dataset_dict.py", line 303, in map
    for k, dataset in self.items()
  File "/venv/lib/python3.6/site-packages/datasets/dataset_dict.py", line 303, in <dictcomp>
    for k, dataset in self.items()
  File "/venv/lib/python3.6/site-packages/datasets/arrow_dataset.py", line 1317, in map
    transformed_shards = [r.get() for r in results]
  File "/venv/lib/python3.6/site-packages/datasets/arrow_dataset.py", line 1317, in <listcomp>
    transformed_shards = [r.get() for r in results]
  File "/venv/lib/python3.6/site-packages/multiprocess/pool.py", line 644, in get
    raise self._value
FileNotFoundError: [Errno 2] No such file or directory: '_00000_of_00060.cache/tmpsvszxtop'
```


The documentation says
```
cache_file_names (`Optional[Dict[str, str]]`, defaults to `None`): Provide the name of a cache file to use to store the
    results of the computation instead of the automatically generated cache file name.
    You have to provide one :obj:`cache_file_name` per dataset in the dataset dictionary.
```
What is expected is simply the name of a file, not a path. The file will be located in the cache directory of the `wikitext` dataset. You can try again with something like
```python
cache_file_names = {k: f'tokenized_and_grouped_{str(k)}' for k in tokenized_datasets}
```

Managed to get `cache_file_names` working and caching works well with it
Had to make a small modification for it to work:
```
cache_file_names = {k: f'tokenized_and_grouped_{str(k)}.arrow' for k in tokenized_datasets}
```

Another comment on `cache_file_names`, it doesn't save the produced cached files in the dataset's cache folder, it requires to give a path to an existing directory for it to work.
I can confirm that this is how it works in `datasets==1.1.3`

Oh yes indeed ! Maybe we need to update the docstring to mention that it is a path

I fixed the docstring. Hopefully this is less confusing now: https://github.com/huggingface/datasets/commit/42ccc0012ba8864e6db1392430100f350236183a

I upgraded to the latest version and I encountered some strange behaviour, the script I posted in the OP doesn't trigger recalculation, however, if I add the following change it does trigger partial recalculation, I am not sure if its something wrong on my machine or a bug:
```
from datasets import load_dataset
from transformers import AutoTokenizer

datasets = load_dataset('wikitext', 'wikitext-103-raw-v1')
tokenizer = AutoTokenizer.from_pretrained('bert-base-uncased', use_fast=True)

column_names = datasets["train"].column_names
text_column_name = "text" if "text" in column_names else column_names[0]
def tokenize_function(examples):
    return tokenizer(examples[text_column_name], return_special_tokens_mask=True)
# CHANGE
print('hello')
# CHANGE

tokenized_datasets = datasets.map(
    tokenize_function,
    batched=True,
...
```
I am using datasets in the `run_mlm.py` script in the transformers examples and I found that if I change the script without touching any of the preprocessing. it still triggers recalculation which is very weird

Edit: accidently clicked the close issue button 

This is because the `group_texts` line definition changes (it is defined 3 lines later than in the previous call). Currently if a function is moved elsewhere in a script we consider it to be different.

Not sure this is actually a good idea to keep this behavior though. We had this as a security in the early development of the lib but now the recursive hashing of objects is robust so we can probably remove that.
Moreover we're already ignoring the line definition for lambda functions.

I opened a PR to change this, let me know what you think.

Sounds great, thank you for your quick responses and help! Looking forward for the next release.

I am having a similar issue where only the grouped files are loaded from cache while the tokenized ones aren't. I can confirm both datasets are being stored to file, but only the grouped version is loaded from cache. Not sure what might be going on. But I've tried to remove all kinds of non deterministic behaviour, but still no luck. Thanks for the help!


```python
    # Datasets
    train = sorted(glob(args.data_dir + '*.{}'.format(args.ext)))
    if args.dev_split >= len(train):
        raise ValueError("Not enough dev files")
    dev = []
    state = random.Random(1001)
    for _ in range(args.dev_split):
        dev.append(train.pop(state.randint(0, len(train) - 1)))

    max_seq_length = min(args.max_seq_length, tokenizer.model_max_length)

    def tokenize_function(examples):
        return tokenizer(examples['text'], return_special_tokens_mask=True)

    def group_texts(examples):
        # Concatenate all texts from our dataset and generate chunks of max_seq_length
        concatenated_examples = {k: sum(examples[k], []) for k in examples.keys()}
        total_length = len(concatenated_examples[list(examples.keys())[0]])
        # Truncate (not implementing padding)
        total_length = (total_length // max_seq_length) * max_seq_length
        # Split by chunks of max_seq_length
        result = {
            k: [t[i : i + max_seq_length] for i in range(0, total_length, max_seq_length)]
            for k, t in concatenated_examples.items()
        }
        return result

    datasets = load_dataset(
        'text', name='DBNL', data_files={'train': train[:10], 'dev': dev[:5]}, 
        cache_dir=args.data_cache_dir)
    datasets = datasets.map(tokenize_function, 
        batched=True, remove_columns=['text'], 
        cache_file_names={k: os.path.join(args.data_cache_dir, f'{k}-tokenized') for k in datasets},
        load_from_cache_file=not args.overwrite_cache)
    datasets = datasets.map(group_texts, 
        batched=True,
        cache_file_names={k: os.path.join(args.data_cache_dir, f'{k}-grouped') for k in datasets},
        load_from_cache_file=not args.overwrite_cache)
```

And this is the log

```
04/26/2021 10:26:59 - WARNING - datasets.builder -   Using custom data configuration DBNL-f8d988ad33ccf2c1
04/26/2021 10:26:59 - WARNING - datasets.builder -   Reusing dataset text (/home/manjavacasema/data/.cache/text/DBNL-f8d988ad33ccf2c1/0.0.0/e16f44aa1b321ece1f87b07977cc5d70be93d69b20486d6dacd62e12cf25c9a5)
100%|██████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 13/13 [00:00<00:00, 21.07ba/s]
100%|██████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 40/40 [00:01<00:00, 24.28ba/s]
04/26/2021 10:27:01 - WARNING - datasets.arrow_dataset -   Loading cached processed dataset at /home/manjavacasema/data/.cache/train-grouped
04/26/2021 10:27:01 - WARNING - datasets.arrow_dataset -   Loading cached processed dataset at /home/manjavacasema/data/.cache/dev-grouped
```


Hi ! What tokenizer are you using ?

It's the ByteLevelBPETokenizer

This error happened to me too, when I tried to supply my own fingerprint to `map()` via the `new_fingerprint` arg.

Edit: realized it was because my path was weird and had colons and brackets and slashes in it, since one of the variable values I included in the fingerprint was a dataset split like "train[:10%]". I fixed it with [this solution](https://stackoverflow.com/a/13593932/2287177) from StackOverflow to just remove those invalid characters from the fingerprint.

Good catch @jxmorris12, maybe we should do additional checks on the valid characters for fingerprints ! Would you like to contribute this ?

I think this can be added here, when we set the fingerprint(s) that are passed `map`:

https://github.com/huggingface/datasets/blob/25bb7c9cbf519fbbf9abf3898083b529e7762705/src/datasets/fingerprint.py#L449-L454

maybe something like
```python
if kwargs.get(fingerprint_name) is None:
    ...
else:
    # In this case, it's the user who specified the fingerprint manually:
    # we need to make sure it's a valid hash
    validate_fingerprint(kwargs[fingerprint_name])
```

Otherwise I can open a PR later

I opened a PR here to add the fingerprint validation: https://github.com/huggingface/datasets/pull/4587

EDIT: merged :)

thank you!