SergeiGKS/huggingface-dataset-issues huggingface dataset:
Issue: `None` converted to `"None"` when loading a dataset

Comment:
Thanks for reporting.

This is actually a breaking change that I think can cause issues when users preprocess their data. String columns used to be nullable. Maybe we can correct https://github.com/huggingface/datasets/pull/3158 to keep the None values and avoid this breaking change ?

EDIT: the other types (bool, int, etc) can also become nullable IMO
Issue: `None` converted to `"None"` when loading a dataset

Comment:
So what would be the best way to handle a feature that can have a null value in some of the instances? So far I used `None`.
Using the empty string won't be a good option, as it can be an actual value in the data and is not the same as not having a value at all.
Issue: `None` converted to `"None"` when loading a dataset

Comment:
Hi @eladsegal,

Use `None`. As @albertvillanova correctly pointed out, this change in conversion was introduced (by mistake) in #3158. To avoid it, install the earlier revision with:
```
pip install git+https://github.com/huggingface/datasets.git@8107844ec0e7add005db0585c772ee20adc01a5e
```

I'm making all the feature types nullable as we speak, and the fix will be merged probably early next week.
Issue: `None` converted to `"None"` when loading a dataset

Comment:
https://github.com/huggingface/datasets/pull/3195 fixed it, we'll do a new release soon :)

For now feel free to install `datasets` from the master branch
Issue: `None` converted to `"None"` when loading a dataset

Comment:
Thanks, but unfortunately looks like it isn't fixed yet 😢 
[notebook for 1.14.0](https://colab.research.google.com/drive/1SV3sFXPJMWSQgbm4pr9Y1Q8OJ4JYKcDo?usp=sharing)
[notebook for master](https://colab.research.google.com/drive/145wDpuO74MmsuI0SVLcI1IswG6aHpyhi?usp=sharing)
Issue: `None` converted to `"None"` when loading a dataset

Comment:
Oh, sorry. I deleted the fix by accident when I was resolving a merge conflict. Let me fix this real quick.
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
After some digging, I found that this is caused by `dill` and using `recurse=True)` when trying to dump the object. The problem also occurs without multiprocessing. I can only find [the following information](https://dill.readthedocs.io/en/latest/dill.html#dill._dill.dumps) about this:

> If recurse=True, then objects referred to in the global dictionary are recursively traced and pickled, instead of the default behavior of attempting to store the entire global dictionary. This is needed for functions defined via exec().

In the utils, this is explicitly enabled

https://github.com/huggingface/datasets/blob/df63614223bf1dd1feb267d39d741bada613352c/src/datasets/utils/py_utils.py#L327-L330

Is this really necessary? Is there a way around it? Also pinging the spaCy team in case this is easy to solve on their end. (I hope so.)
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
Hi ! Thanks for reporting

Yes `recurse=True` is necessary to be able to hash all the objects that are passed to the `map` function

EDIT: hopefully this object can be serializable soon, but otherwise we can consider adding more control to the user on how to hash objects that are not serializable (as mentioned in https://github.com/huggingface/datasets/issues/3044#issuecomment-948818210)
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
I submitted a PR to spacy that should fix this issue (linked above). I'll leave this open until that PR is merged.
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
@lhoestq After some testing I find that even with the updated spaCy, no cache files are used. I do not get any warnings though, but I can see that map is run every time I run the code. Do you have thoughts about why? If you want to try the tests below, make sure to install spaCy from [here](https://github.com/BramVanroy/spaCy) and installing the base model with `python -m spacy download en_core_web_sm`.

```python
from functools import partial
from pathlib import Path

import spacy
from datasets import Dataset
import datasets
datasets.logging.set_verbosity_debug()

def tokenize(nlp, l):
    return {"tok": [t.text for t in nlp(l["text"])]}

def main():
    fin = r"some/file/with/many/lines"
    lines = Path(fin).read_text(encoding="utf-8").splitlines()
    nlp = spacy.load("en_core_web_sm")
    ds = Dataset.from_dict({"text": lines, "text_id": list(range(len(lines)))})
    tok = partial(tokenize, nlp)
    ds = ds.map(tok, load_from_cache_file=True)
    print(ds[0:2])

if __name__ == '__main__':
    main()
```

... or with load_dataset (here I get the message that `load_dataset` can reuse the dataset, but still I see all samples being processed via the tqdm progressbar):

```python
from functools import partial

import spacy
from datasets import load_dataset
import datasets
datasets.logging.set_verbosity_debug()

def tokenize(nlp, sample):
    return {"tok": [t.text for t in nlp(sample["text"])]}

def main():
    fin = r"some/file/with/many/lines"
    nlp = spacy.load("en_core_web_sm")
    tok_func = partial(tokenize, nlp)
    ds = load_dataset('text', data_files=fin)
    ds = ds["train"].map(tok_func)
    print(ds[0:2])

if __name__ == '__main__':
    main()
```
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
It looks like every time you load `en_core_web_sm` you get a different python object:
```python
import spacy
from datasets.fingerprint import Hasher

nlp1 = spacy.load("en_core_web_sm")
nlp2 = spacy.load("en_core_web_sm")
Hasher.hash(nlp1), Hasher.hash(nlp2)
# ('f6196a33882fea3b', 'a4c676a071f266ff')
```
Here is a list of attributes that have different hashes for `nlp1` and `nlp2`:
- tagger
- parser
- entity
- pipeline (it's the list of the three attributes above)

I just took a look at the tagger for example and I found subtle differences (there may be other differences though):
```python
nlp1.tagger.model.tok2vec.embed.id, nlp2.tagger.model.tok2vec.embed.id
# (1721, 2243)
```

We can try to find all the differences and find the best way to hash those objects properly
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
Thanks for searching! I went looking, and found that this is an implementation detail of thinc

https://github.com/explosion/thinc/blob/68691e303ae68cae4bc803299016f1fc064328bf/thinc/model.py#L96-L98

Presumably (?) exactly to distinguish between different parts in memory when multiple models are loaded. Do not think that this can be changed on their end - but I will ask what exactly it is for (I'm curious).

Do you think it is overkill to write something into the hasher explicitly to deal with spaCy models? It seems like something that is beneficial to many, but I do not know if you are open to adding third-party-specific ways to deal with this. If you are, I can have a look for this specific case how we can ignore `thinc.Model.id` from the hasher.
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
It can be even simpler to hash the bytes of the pipeline instead
```python
nlp1.to_bytes() == nlp2.to_bytes()  # True
```

IMO we should integrate the custom hashing for spacy models into `datasets` (we use a custom Pickler for that).
What could be done on Spacy's side instead (if they think it's nice to have) is to implement a custom pickling for these classes using `to_bytes`/`from_bytes` to have deterministic pickle dumps.

Finally I think it would be nice in the future to add an API to let `datasets` users control this kind of things. Something like being able to define your own hashing if you use complex objects.
```python
@datasets.register_hash(spacy.language.Language)
def hash_spacy_language(nlp):
    return Hasher.hash(nlp.to_bytes())
```
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
I do not quite understand what you mean. as far as I can tell, using `to_bytes` does a pickle dump behind the scene (with `srsly`), recursively using `to_bytes` on the required objects. Therefore, the result of `to_bytes` is a deterministic pickle dump AFAICT. Or do you mean that you wish that using your own pickler and running `dumps(nlp)` should also be deterministic? I guess that would require `__setstate__` and `__getstate__` methods on all the objects that have to/from_bytes. I'll have a listen over at spaCy what they think, and if that would solve the issue. I'll try this locally first, if I find the time.

I agree that having the option to use a custom hasher would be useful. I like your suggestion!

EDIT: after trying some things and reading through their API, it seems that they explicitly do not want this. https://spacy.io/usage/saving-loading#pipeline

> When serializing the pipeline, keep in mind that this will only save out the binary data for the individual components to allow spaCy to restore them – not the entire objects. This is a good thing, because it makes serialization safe. But it also means that you have to take care of storing the config, which contains the pipeline configuration and all the relevant settings.

Best way forward therefore seems to implement the ability to specify a hasher depending on the objects that are pickled, as you suggested. I can work on this if that is useful. I could use some pointers as to how you would like to implement the `register_hash` functionality though. I assume using `catalogue` over at Explosion might be a good starting point.
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
Interestingly, my PR does not solve the issue discussed above. The `tokenize` function hash is different on every run, because for some reason `nlp.__call__` has a different hash every time. The issue therefore seems to run much deeper than I thought. If you have any ideas, I'm all ears.

```shell
git clone https://github.com/explosion/spaCy.git
cd spaCy/
git checkout cab9209c3dfcd1b75dfe5657f10e52c4d847a3cf
cd ..

git clone https://github.com/BramVanroy/datasets.git
cd datasets
git checkout registry
pip install -e .
pip install ../spaCy
spacy download en_core_web_sm
```

```python
import spacy

from datasets import load_dataset
from datasets.fingerprint import Hasher
from datasets.utils.registry import hashers

@hashers.register(spacy.Language)
def hash_spacy_language(nlp):
    return Hasher.hash(nlp.to_bytes())

def main():
    fin = r"your/large/file"
    nlp = spacy.load("en_core_web_sm")
    # This is now always the same yay!
    print(Hasher.hash(nlp))

    def tokenize(l):
        return {"tok": [t.text for t in nlp(l["text"])]}

    ds =  load_dataset("text", data_files=fin)
    # But this is not...
    print(Hasher.hash(tokenize))
    # ... because of this
    print(Hasher.hash(nlp.__call__))
    ds = ds["train"].map(tokenize)
    print(ds[0:2])


if __name__ == '__main__':
    main()
```
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
Hi ! I just answered in your PR :) In order for your custom hashing to be used for nested objects, you must integrate it into our recursive pickler that we use for hashing.
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
I don't quite understand the design constraints of `datasets` or the script that you're running, but my usual advice is to avoid using pickle unless you _absolutely_ have to. So for instance instead of doing your `partial` over the `nlp` object itself, can you just pass the string `en_core_web_sm` in? This will mean calling `spacy.load()` inside the work function, but this is no worse than having to call `pickle.load()` on the contents of the NLP object anyway -- in fact you'll generally find `spacy.load()` faster, apart from the disk read.

If you need to pass in the bytes data and don't want to read from disk, you could do something like this:

```
msg = (nlp.lang, nlp.to_bytes())

def unpack(lang, bytes_data):
    return spacy.blank(lang).from_bytes(bytes_data)
```

I think that should probably work: the Thinc `model.to_dict()` method (which is used by the `model.to_bytes()` method) doesn't pack the model's ID into the message, so the `nlp.to_bytes()` that you get shouldn't be affected by the global IDs. So you should get a clean message from `nlp.to_bytes()` that doesn't depend on the global state.
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
Hi Matthew, thanks for chiming in! We are currently implementing exactly what you suggest: `to_bytes()` as a default before pickling - but we may prefer `to_dict` to avoid double dumping.

`datasets` uses pickle dumps (actually dill) to get unique representations of processing steps (a "fingerprint" or hash). So it never needs to re-load that dump - it just needs its value to create a hash. If a fingerprint is identical to a cached fingerprint, then the result can be retrieved from the on-disk cache. (@lhoestq or @mariosasko can correct me if I'm wrong.)

I was experiencing the issue that parsing with spaCy gave me a different fingerprint on every run of the script and thus it could never load the processed dataset from cache. At first I thought the reason was that spaCy Language objects were not picklable with recursive dill, but even after [adjusting for that](https://github.com/explosion/spaCy/pull/9593) the issue persisted. @lhoestq found that this is due to the changing `id`, which you discussed [here](https://github.com/explosion/spaCy/discussions/9609#discussioncomment-1661081). So yes, you are right. On the surface there simply seems to be an incompatibility between `datasets` default caching functionality as it is currently implemented and `spacy.Language`.

The [linked PR](https://github.com/huggingface/datasets/pull/3224) aims to remedy that, though. Up to now I have put some effort into making it easier to define your own "pickling" function for a given type (and optionally any of its subclasses). That allows us to tell `datasets` that instead of doing `dill.save(nlp)` (non-deterministic), to use `dill.save(nlp.to_bytes())` (deterministic). When I find some more time, the PR [will be expanded](https://github.com/huggingface/datasets/pull/3224#issuecomment-968958528) to improve the user-experience a bit and add a built-in function to pickle `spacy.Language` as one of the defaults (using `to_bytes()`).
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
Is there a workaround for this? maybe by explicitly requesting datasets to cache the result of `.map()`?
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
Hi ! If your function is not picklable, then the fingerprint of the resulting dataset can't be computed. The fingerprint is a hash that is used by the cache to reload previously computed datasets: the dataset file is named `cache-<fingerprint>.arrow` in your dataset's cache directory.

As a workaround you can set the fingerprint that is going to be used by the cache:
```python
result = my_dataset.map(func, new_fingerprint=new_fingerprint)
```
Any future call to `map` with the same `new_fingerprint` will reload the result from the cache.

**Be careful using this though: if you change your `func`, be sure to change the `new_fingerprint` as well.**
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
I've been having an issue that might be related to this when trying to pre-tokenize a corpus and caching it for using it later in the pre-training of a RoBERTa model. I always get the following warning:

```
Dataset text downloaded and prepared to /gpfswork/rech/project/user/.cache/hf-datasets/text/default-1850886023af0077/0.0.0/acc32f2f2ef863c93c2f30c52f7df6cc9053a1c2230b8d7da0d210404683ca08. Subsequent calls will reuse this data.
Parameter 'function'=<function encode_dataset.<locals>.<lambda> at 0x14a92157b280> of the transform datasets.arrow_dataset.Dataset.filter@2.0.1 couldn't be hashed properly, a random hash was used instead. Make sure your transforms and parameters are serializable with pickle or dill for the dataset fingerprinting and caching to work. If you reuse this transform, the caching mechanism will consider it to be different from the previous calls and recompute everything. This warning is only showed once. Subsequent hashing failures won't be showed.
```

And when I launch the pre-training the pre-tokenized corpus is not found and it is tokenized again, which makes me waste precious GPU hours.

For me, the workaround was downgrading `dill` and `multiprocess` to the following versions:

```
dill             0.3.4
multiprocess     0.70.12.2             
```
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
> Hi ! If your function is not picklable, then the fingerprint of the resulting dataset can't be computed. The fingerprint is a hash that is used by the cache to reload previously computed datasets: the dataset file is named `cache-<fingerprint>.arrow` in your dataset's cache directory.
> 
> As a workaround you can set the fingerprint that is going to be used by the cache:
> 
> ```python
> result = my_dataset.map(func, new_fingerprint=new_fingerprint)
> ```
> 
> Any future call to `map` with the same `new_fingerprint` will reload the result from the cache.
> 
> **Be careful using this though: if you change your `func`, be sure to change the `new_fingerprint` as well.**

Is the argument `new_fingerprint` available for datasetDict ? I can only use it on arrow datasets but might be useful to generalize it to DatasetDict's map as well ?  @lhoestq
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
> I've been having an issue that might be related to this when trying to pre-tokenize a corpus and caching it for using it later in the pre-training of a RoBERTa model. I always get the following warning:
> 
> ```
> Dataset text downloaded and prepared to /gpfswork/rech/project/user/.cache/hf-datasets/text/default-1850886023af0077/0.0.0/acc32f2f2ef863c93c2f30c52f7df6cc9053a1c2230b8d7da0d210404683ca08. Subsequent calls will reuse this data.
> Parameter 'function'=<function encode_dataset.<locals>.<lambda> at 0x14a92157b280> of the transform datasets.arrow_dataset.Dataset.filter@2.0.1 couldn't be hashed properly, a random hash was used instead. Make sure your transforms and parameters are serializable with pickle or dill for the dataset fingerprinting and caching to work. If you reuse this transform, the caching mechanism will consider it to be different from the previous calls and recompute everything. This warning is only showed once. Subsequent hashing failures won't be showed.
> ```
> 
> And when I launch the pre-training the pre-tokenized corpus is not found and it is tokenized again, which makes me waste precious GPU hours.
> 
> For me, the workaround was downgrading `dill` and `multiprocess` to the following versions:
> 
> ```
> dill             0.3.4
> multiprocess     0.70.12.2             
> ```

This worked for me - thanks!
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
I see this has just been closed - it seems quite relevant to another tokenizer I have been trying to use, the `vinai/phobert` family of tokenizers

https://huggingface.co/vinai/phobert-base
https://huggingface.co/vinai/phobert-large

I ran into an issue where a large dataset took several hours to tokenize, the process hung, and I was unable to use the cached version of the tokenized data:

https://discuss.huggingface.co/t/cache-parallelize-long-tokenization-step/25791/3

I don't see any way to specify the hash of the tokenizer or the fingerprint of the tokenized data to use, so is the tokenized dataset basically lost at this point?  Is there a good way to avoid this happening again if I retokenize the data?
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
In your case it looks like the job failed before caching the data - maybe one of the processes crashed
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
Interesting.  Thanks for the observation.  Any suggestions on how to start tracking that down?  Perhaps run it singlethreaded and see if it crashes?
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
You can monitor your RAM and disk space in case a process dies from OOM or disk full, and when it hangs you can check how many processes are running. IIRC there are other start methods for multiprocessing in python that may show an error message if a process dies.

Running on a single process can also help debugging this indeed
Issue: "Property couldn't be hashed properly" even though fully picklable

Comment:
Hi @tung-msol could you open a new issue and share the error you got and the map function you used ?
Issue: More control over TQDM when using map/filter with multiple processes

Comment:
Hi,

It's hard to provide an API that would cover all use-cases with tqdm in this project.

However, you can make it work by defining a custom decorator (a bit hacky tho) as follows:
```python
import datasets

def progress_only_on_rank_0(func):
    def wrapper(*args, **kwargs):
        rank = kwargs.get("rank")
        disable_tqdm = kwargs.get("disable_tqdm", False)
        disable_tqdm = True if rank is not None and rank > 0 else disable_tqdm
        kwargs["disable_tqdm"] = disable_tqdm
        return func(*args, **kwargs)
    return wrapper
    
datasets.Dataset._map_single = progress_only_on_rank_0(datasets.Dataset._map_single)
``` 

EDIT: Ups, closed by accident.

Thanks for the provided links. `Trainer` requires this for training in multi-node distributed setting. However, `Dataset.map` doesn't support that yet.

Do you have an API for this in mind? `Dataset.map` is already bloated with the arguments, so IMO it's not a good idea to add a new arg there.
Issue: More control over TQDM when using map/filter with multiple processes

Comment:
Inspiration may be found at `transformers`.

https://github.com/huggingface/transformers/blob/4a394cf53f05e73ab9bbb4b179a40236a5ffe45a/src/transformers/trainer.py#L1231-L1233

To get unique IDs for each worker, see https://stackoverflow.com/a/10192611/1150683
Issue: `SystemError 15` thrown in `Dataset.__del__` when using `Dataset.map()` with `num_proc>1`

Comment:
NB: even if the error is raised, the dataset is successfully cached. So restarting the script after every `map()` allows to ultimately run the whole preprocessing. But this prevents to realistically run the code over multiple nodes.
Issue: `SystemError 15` thrown in `Dataset.__del__` when using `Dataset.map()` with `num_proc>1`

Comment:
Hi,

It's not easy to debug the problem without the script. I may be wrong since I'm not very familiar with PyTorch Lightning, but shouldn't you preprocess the data in the `prepare_data` function of `LightningDataModule` and not in the `setup` function.
As you can't modify the module state in `prepare_data` (according to the docs), use the `cache_file_name` argument in `Dataset.map` there, and reload the processed data in `setup` with `Dataset.from_file(cache_file_name)`. If `num_proc>1`, check the docs on the `suffix_template` argument of `Dataset.map` to get an idea what the final `cache_file_names` are going to be.

Let me know if this helps.