SergeiGKS/huggingface-dataset-issues huggingface dataset:
Issue: Unexpected behavior doing Split + Filter

Comment:
Hi ! This is an issue with `datasets` 1.12. Sorry for the inconvenience. Can you update to `>=1.13` ?
see https://github.com/huggingface/datasets/issues/3190

Maybe we should also backport the bug fix to `1.12` (in a new version `1.12.2`)
Issue: Add `__add__()`, `__iadd__()` and similar to `Dataset` class

Comment:
I was going through the codebase, and I believe the implementation of __add__() and __iadd__() will be similar to concatenate_datasets() after the elimination of code for arguments other than the list of datasets (info, split, axis). 
(Assuming elimination of axis means concatenating over axis 1.)
Issue: Add `__add__()`, `__iadd__()` and similar to `Dataset` class

Comment:
Most data frame libraries (Polars, Pandas, ...) override `__add__` to perform (mathematical) summation, so having different behavior here is a bad idea.
Issue: JSONDecodeError with HuggingFace dataset viewer

Comment:
Hi ! I think the issue comes from the dataset_infos.json file: it has the "flat" field twice.

Can you try deleting this file and regenerating it please ?
Issue: JSONDecodeError with HuggingFace dataset viewer

Comment:
Thanks! That fixed that, but now I am getting:
Server Error
Status code:   400
Exception:     KeyError
Message:       'feature'

I checked the dataset_infos.json and pubmed_neg.py script, I don't use 'feature' anywhere as a key. Is the dataset viewer expecting that I do?
Issue: JSONDecodeError with HuggingFace dataset viewer

Comment:
It seems that the `feature` key is missing from some feature type definition in your dataset_infos.json:
```json
			"tokens": {
				"dtype": "list",
				"id": null,
				"_type": "Sequence"
			},
			"tags": {
				"dtype": "list",
				"id": null,
				"_type": "Sequence"
			}
```
They should be
```json
			"tokens": {
				"dtype": "list",
				"id": null,
				"_type": "Sequence"
                                 "feature": {"dtype": "string", "id": null, "_type": "Value"}
			},
			"tags": {
				"dtype": "list",
				"id": null,
				"_type": "Sequence",
                                 "feature": {"num_classes": 5, "names": ["-", "S", "H", "N", "C"], "names_file": null, "id": null, "_type": "ClassLabel"}
			}
```

Note that you can generate the dataset_infos.json automatically to avoid mistakes:
```bash
datasets-cli test ./path/to/dataset --save_infos
```
Issue: HF_DATASETS_OFFLINE=1 didn't stop datasets.builder from downloading 

Comment:
Hi ! Indeed it says "downloading and preparing" but in your case it didn't need to download anything since you used local files (it would have thrown an error otherwise). I think we can improve the logging to make it clearer in this case
Issue: HF_DATASETS_OFFLINE=1 didn't stop datasets.builder from downloading 

Comment:
@lhoestq Thank you for explaining. I am sorry but I was not clear about my intention. I didn't want to kill internet traffic; I wanted to kill all write activity. In other words, you can imagine that my storage has only read access but crashes on write.

When run_clm.py is invoked with the same parameters, the hash in the cache directory "datacache/trainpy.v2/json/default-471372bed4b51b53/0.0.0/..." doesn't change, and my job can load cached data properly. This is great.

Unfortunately, when params change (which happens sometimes), the hash changes and the old cache is invalid. datasets builder would create a new cache directory with the new hash and create JSON builder there, even though every JSON builder is the same. I didn't find a way to avoid such behavior.

This problem can be resolved when using datasets.map() for tokenizing and grouping text. This function allows me to specify output filenames with --cache_file_names, so that the cached files are always valid.

This is the code that I used to freeze cache filenames for tokenization. I wish I could do the same to datasets.load_dataset()
```
        tokenized_datasets = raw_datasets.map(
            tokenize_function,
            batched=True,
            num_proc=data_args.preprocessing_num_workers,
            remove_columns=column_names,
            load_from_cache_file=not data_args.overwrite_cache,
            desc="Running tokenizer on dataset",
            cache_file_names={k: os.path.join(model_args.cache_dir, f'{k}-tokenized') for k in raw_datasets},
        )
```
Issue: HF_DATASETS_OFFLINE=1 didn't stop datasets.builder from downloading 

Comment:
Hi ! `load_dataset` may re-generate your dataset if some parameters changed indeed. If you want to freeze a dataset loaded with `load_dataset`, I think the best solution is just to save it somewhere on your disk with `.save_to_disk(my_dataset_dir)` and reload it with `load_from_disk(my_dataset_dir)`. This way you will be able to reload the dataset without having to run `load_dataset`
Issue: Align the Dataset and IterableDataset processing API

Comment:
Yes I agree, these should be as aligned as possible. Maybe we can also check the feedback in the survey at http://hf.co/oss-survey and see if people mentioned related things on the API (in particular if we go the breaking change way, it would be good to be sure we are taking the right direction for the community).
Issue: Align the Dataset and IterableDataset processing API

Comment:
I like this proposal.

> There is also an important difference in terms of behavior:
Dataset.map adds new columns (with dict.update)
BUT
IterableDataset discards previous columns (it overwrites the dict)
IMO the two methods should have the same behavior. This would be an important breaking change though.

> The main breaking change would be the change of behavior of IterableDataset.map, because currently it discards all the previous columns instead of keeping them.

Yes, this behavior of `IterableDataset.map` was surprising to me the first time I used it because I was expecting the same behavior as `Dataset.map`, so I'm OK with the breaking change here.

> IterableDataset only supports "torch" (it misses tf, jax, pandas, arrow) and is missing the parameters: columns, output_all_columns and format_kwargs

\+  it's also missing the actual formatting code (we return unformatted tensors)
> We could have a completely aligned map method if both methods were lazy by default, but this is a very big breaking change so I'm not sure we can consider doing that.

> For information, TFDS does lazy map by default, and has an additional .cache() method.

If I understand this part correctly, the idea would be for `Dataset.map` to behave similarly to `Dataset.with_transform` (lazy processing) and to have an option to cache processed data (with `.cache()`). This idea is really nice because it can also be applied to `IterableDataset` to fix https://github.com/huggingface/datasets/issues/3142 (again we get the aligned APIs). However, this change would break a lot of things, so I'm still not sure if this is a step in the right direction (maybe it's OK for Datasets 2.0?) 
> If the two APIs are more aligned it would be awesome for the examples in transformers, and it would create a satisfactory experience for users that want to switch from one mode to the other.

Yes, it would be amazing to have an option to easily switch between these two modes.

I agree with the rest.
Issue: Align the Dataset and IterableDataset processing API

Comment:
> If I understand this part correctly, the idea would be for Dataset.map to behave similarly to Dataset.with_transform (lazy processing) and to have an option to cache processed data (with .cache()). This idea is really nice because it can also be applied to IterableDataset to fix #3142 (again we get the aligned APIs). However, this change would break a lot of things, so I'm still not sure if this is a step in the right direction (maybe it's OK for Datasets 2.0?)

Yea this is too big of a change in my opinion. Anyway it's fine as it is right now with streaming=lazy and regular=eager.
Issue: Align the Dataset and IterableDataset processing API

Comment:
Yes indeed, thanks. I added it to the list of methods to align in the first post
Issue: Align the Dataset and IterableDataset processing API

Comment:
I just encountered the problem of the missing `fn_kwargs` parameter in the `map` method. I am commenting to give a workaround in case someone has the same problem and does not find a solution.
You can wrap your function call inside a class that contains the other parameters needed by the function called by map, like this:

```python
def my_func(x, y, z):
    # Do things

class MyFuncWrapper:
    def __init__(self, y, z):
        self.y = y
        self.z = z

    def __call__(self, x):
        return my_func(x, self.y, self.z)
```

Then, give an instance of the `MyFuncWrapper` to the map function.
Issue: Align the Dataset and IterableDataset processing API

Comment:
The main differences have been addressed (map, formatting) but there are still a few things to implement like Dataset.take, Dataset.skip, IterableDataset.set_format, IterableDataset.formatted_as, IterableDataset.reset_format.

The rest cannot be implemented for the general case. E.g. train_test_split and select can only work on an iterable dataset if the underlying dataset format allows it (we need to know the number of rows and have some sort of random access)
Issue: datasets keeps reading from cached files, although I disabled it

Comment:
Hi ! What version of `datasets` are you using ? Can you also provide the logs you get before it raises the error ?
Issue: Unable to resolve any data file after loading once

Comment:
Hi ! `load_dataset` accepts as input either a local dataset directory or a dataset name from the Hugging Face Hub.

So here you are getting this error the second time because it tries to load the local `wiki_dpr` directory, instead of `wiki_dpr` from the Hub. It doesn't work since it's a **cache** directory, not a **dataset** directory in itself.

To fix that you can use another cache directory like `cache_dir="/data2/whr/lzy/open_domain_data/retrieval/cache"`
Issue: Getting configs names takes too long

Comment:
It looks like it's currently calling `HfFileSystem.ls()` ~8 times at the root and for each subdirectory:
- ""
- "en.noblocklist"
- "en.noclean"
- "en"
- "multilingual"
- "realnewslike"

Currently `ls` is slow because it iterates on all the files inside the repository.

An easy optimization would be to cache the result of each call to `ls`.
We can also optimize `ls` by using a tree structure per directory instead of a list of all the files.
Issue: data duplicate when setting num_works > 1 with streaming data

Comment:
Hi ! Thanks for reporting :)

When using a PyTorch's data loader with `num_workers>1` and an iterable dataset, each worker streams the exact same data by default, resulting in duplicate data when iterating using the data loader.

We can probably fix this in `datasets` by checking `torch.utils.data.get_worker_info()` which gives the worker id if it happens.
Issue: data duplicate when setting num_works > 1 with streaming data

Comment:
> Hi ! Thanks for reporting :)
> 
> When using a PyTorch's data loader with `num_workers>1` and an iterable dataset, each worker streams the exact same data by default, resulting in duplicate data when iterating using the data loader.
> 
> We can probably fix this in `datasets` by checking `torch.utils.data.get_worker_info()` which gives the worker id if it happens.
Hi ! Thanks for reply

Do u have some plans to fix the problem?
Issue: data duplicate when setting num_works > 1 with streaming data

Comment:
Isn’t that somehow a bug on PyTorch side? (Just asking because this behavior seems quite general and maybe not what would be intended)
Issue: data duplicate when setting num_works > 1 with streaming data

Comment:
From PyTorch's documentation [here](https://pytorch.org/docs/stable/data.html#dataset-types):

> When using an IterableDataset with multi-process data loading. The same dataset object is replicated on each worker process, and thus the replicas must be configured differently to avoid duplicated data. See [IterableDataset](https://pytorch.org/docs/stable/data.html#torch.utils.data.IterableDataset) documentations for how to achieve this.

It looks like an intended behavior from PyTorch

As suggested in the [docstring of the IterableDataset class](https://pytorch.org/docs/stable/data.html#torch.utils.data.IterableDataset), we could pass a `worker_init_fn` to the DataLoader to fix this. It could be called `streaming_worker_init_fn` for example.

However, while this solution works, I'm worried that many users simply don't know about this parameter and just start their training with duplicate data without knowing it. That's why I'm more in favor of integrating the check on the worker id directly in `datasets` in our implementation of `IterableDataset.__iter__`.
Issue: data duplicate when setting num_works > 1 with streaming data

Comment:
Hi there @lhoestq @cloudyuyuyu 
I met that problem recently, and #4375 is really useful because I finally found out I am training with duplicate data.
However, in multi-GPU training, I'm using DDP mode and IterableDataset, which still yields duplicate data for each progress. And this is dangerous because users maybe not realize this behavior.
Issue: data duplicate when setting num_works > 1 with streaming data

Comment:
If the worker_info.id is unique per process it should work fine, could you check that they're unique ?

The code to get the worker_info in each worker is `torch.utils.data.get_worker_info()`
Issue: data duplicate when setting num_works > 1 with streaming data

Comment:
test.py
```python
import json
import os

import torch
from torch.utils.data import IterableDataset, DataLoader
from transformers import PreTrainedTokenizer, TrainingArguments

from common.arguments import DataTrainingArguments, ModelArguments


class MyIterableDataset(IterableDataset):
    def __iter__(self):
        worker_info = torch.utils.data.get_worker_info()
        print(worker_info)
        return iter(range(3))


if __name__ == '__main__':
    dataset = MyIterableDataset()
    dataloader = DataLoader(dataset, num_workers=1)
    for i in dataloader:
        print(i)

```


```sh
$ python3 -m torch.distributed.launch \
  --nproc_per_node=2 test.py
WorkerInfo(id=0, num_workers=1, seed=5545685212307804959, dataset=<__main__.MyIterableDataset object at 0x7f92648cf6a0>)
WorkerInfo(id=0, num_workers=1, seed=3174108029709729025, dataset=<__main__.MyIterableDataset object at 0x7f19ab961670>)
tensor([0])
tensor([1])
tensor([2])
tensor([0])
tensor([1])
tensor([2])
```

@lhoestq they are not unique
Issue: data duplicate when setting num_works > 1 with streaming data

Comment:
It looks like a bug from pytorch no ? How can we know which data should go in which process when using DDP ?

I guess we need to check `torch.distributed.get_world_size()` and `torch.distributed.get_rank()` as well. Not fan of the design here tbh, but that's how it is
Issue: data duplicate when setting num_works > 1 with streaming data

Comment:
> It looks like a bug from pytorch no ? How can we know which data should go in which process when using DDP ?
> 
> I guess we need to check `torch.distributed.get_world_size()` and `torch.distributed.get_rank()` as well. Not fan of the design here tbh, but that's how it is

Maybe we should document it?
Issue: data duplicate when setting num_works > 1 with streaming data

Comment:
hmm actually let me open a new issue on DDP - original post was for single node
Issue: Error about load_metric

Comment:
Hi ! I wasn't able to reproduce your error.

Can you try to clear your cache at `~/.cache/huggingface/modules` and try again ?
Issue: `.to_json` is extremely slow after `.select`

Comment:
Hi ! It's slower indeed because a datasets on which `select`/`shard`/`train_test_split`/`shuffle` has been called has to do additional steps to retrieve the data of the dataset table in the right order.

Indeed, if you call `dataset.select([0, 5, 10])`, the underlying table of the dataset is not altered to keep the examples at index 0, 5, and 10. Instead, an indices mapping is added on top of the table, that says that the first example is at index 0, the second at index 5 and the last one at index 10.

Therefore accessing the examples of the dataset is slower because of the additional step that uses the indices mapping.

The step that takes the most time is to query the dataset table from a list of indices here:

https://github.com/huggingface/datasets/blob/047dc756ed20fbf06e6bcaf910464aba0e20610a/src/datasets/formatting/formatting.py#L61-L63

In your case it can be made significantly faster by checking if the indices are contiguous. If they're contiguous, we could pass a python `slice` or `range` instead of a list of integers to `_query_table`. This way `_query_table` will do only one lookup to get the queried batch instead of `batch_size` lookups.

Given that calling `select` with contiguous indices is a common use case I'm in favor of implementing such an optimization :)
Let me know what you think
Issue: `.to_json` is extremely slow after `.select`

Comment:
Hi, thanks for the response!
I still don't understand why it is so much slower than iterating and saving:
```python
from datasets import load_dataset

original = load_dataset("squad", split="train")
original.to_json("from_original.json")  # Takes 0 seconds

selected_subset1 = original.select([i for i in range(len(original))])
selected_subset1.to_json("from_select1.json")  # Takes 99 seconds

selected_subset2 = original.select([i for i in range(int(len(original) / 2))])
selected_subset2.to_json("from_select2.json")  # Takes 47 seconds

selected_subset3 = original.select([i for i in range(len(original)) if i % 2 == 0])
selected_subset3.to_json("from_select3.json")  # Takes 49 seconds

import json
import time
def fast_to_json(dataset, path):
    start = time.time()
    with open(path, mode="w") as f:
        for example in dataset:
            f.write(json.dumps(example, separators=(',', ':')) + "\n")
    end = time.time()
    print(f"Saved {len(dataset)} examples to {path} in {end - start} seconds.")

fast_to_json(original, "from_original_fast.json")
fast_to_json(selected_subset1, "from_select1_fast.json")
fast_to_json(selected_subset2, "from_select2_fast.json")
fast_to_json(selected_subset3, "from_select3_fast.json")
```
```
Saved 87599 examples to from_original_fast.json in 8 seconds.
Saved 87599 examples to from_select1_fast.json in 10 seconds.
Saved 43799 examples to from_select2_fast.json in 6 seconds.
Saved 43800 examples to from_select3_fast.json in 5 seconds.
```
Issue: `.to_json` is extremely slow after `.select`

Comment:
There are slight differences between what you're doing and what `to_json` is actually doing.
In particular `to_json` currently converts batches of rows (as an arrow table) to a pandas dataframe, and then to JSON Lines. From your benchmark it looks like it's faster if we don't use pandas.

Thanks for investigating, I think we can optimize `to_json` significantly thanks to your test.
Issue: `.to_json` is extremely slow after `.select`

Comment:
Thanks for your observations, @eladsegal! I spent some time with this and tried different approaches. Turns out that https://github.com/huggingface/datasets/blob/bb13373637b1acc55f8a468a8927a56cf4732230/src/datasets/io/json.py#L100 is giving the problem when we use `to_json` after `select`. This is when `indices` parameter in `query_table` is not `None` (if it is `None` then `to_json` should work as expected)

In order to circumvent this problem, I found out instead of doing Arrow Table -> Pandas-> JSON we can directly go to JSON by using `to_pydict()` which is a little slower than the current approach but at least `select` works properly now. Lmk what you guys think of it @lhoestq, @eladsegal?
Issue: `.to_json` is extremely slow after `.select`

Comment:
Posting it in @eladsegal's format:

For `squad`:
Saving examples using current `to_json` in 3.63 secs
Saving examples to `from_select1_fast.json` in 5.00 secs
Saving examples to `from_select2_fast.json` in 2.45 secs
Saving examples to `from_select3_fast.json` in 2.50 secs

For `squad_v2`:
Saving examples using current `to_json` in 5.26 secs
Saving examples to `from_select1_fast.json` in 7.54 secs
Saving examples to `from_select2_fast.json` in 3.80 secs
Saving examples to `from_select3_fast.json` in 3.67 secs
Issue: disaster_response_messages unavailable

Comment:
Hi, thanks for reporting! This is a duplicate of https://github.com/huggingface/datasets/issues/3240. We are working on a fix.
Issue: Non-deterministic tests: CI tests randomly fail

Comment:
I think it might come from two different issues:
1. Google Drive is an unreliable host, mainly because of quota limitations
2. the staging environment can sometimes raise some errors

For Google Drive tests we could set up some retries with backup URLs if necessary I guess.
For staging on the other hand, I guess we can investigate what causes this and discuss with the back-end team
Issue: Improve Wikipedia loading script

Comment:
Thanks! See https://public.paws.wmcloud.org/User:Isaac_(WMF)/HuggingFace%20Wikipedia%20Processing.ipynb for more implementation details / some data around the overhead induced by adding the extra preprocessing steps (stripping link prefixes and magic words)
Issue: Add URL field to Wikimedia dataset instances: wikipedia,...

Comment:
@geohci, I think the field "url" does not appear in the Wikimedia dumps. Therefore I guess we should generate it, using the "title" field and making some transformation of it (replacing spaces with underscores) and prepending the domain (created using the language)?
Issue: Add URL field to Wikimedia dataset instances: wikipedia,...

Comment:
Indeed:

> To re-distribute text on Wikipedia in any form, provide credit to the authors either by including a) a [hyperlink](https://en.wikipedia.org/wiki/Hyperlink) (where possible) or [URL](https://en.wikipedia.org/wiki/URL) to the page or pages you are re-using, b) a hyperlink (where possible) or URL to an alternative, stable online copy which is freely accessible, which conforms with the license, and which provides credit to the authors in a manner equivalent to the credit given on this website, or c) a list of all authors. (Any list of authors may be filtered to exclude very small or irrelevant contributions.) This applies to text developed by the Wikipedia community. Text from external sources may attach additional attribution requirements to the work, which should be indicated on an article's face or on its talk page. For example, a page may have a banner or other notation indicating that some or all of its content was originally published somewhere else. Where such notations are visible in the page itself, they should generally be preserved by re-users.

source: https://en.wikipedia.org/wiki/Wikipedia:Copyrights

I guess it's fine to add the URL field - it can be constructed easily from the title page IIRC.
Issue: Add URL field to Wikimedia dataset instances: wikipedia,...

Comment:
yep, sorry forgot that that wasn't already in the dumps. specifically `f"https://{language}.wikipedia.org/wiki/{title.replace(' ', '_')}` should do it