t20062443/AT1ds huggingface dataset:
gregburman (NONE) commented on 2020-06-19 12:49:02:
Hi @lhoestq 

Wow, thanks so much, that's **really** incredible! I was considering looking at creating my own Beam Dataset, as per the doc you linked, but instead opted to process the data myself using `wikiextractor`. However, now that this is available, I'll definitely switch across and use it.

Thanks so much for the incredible work, this really helps out our team considerably!

Have a great (and well-deserved ;) weekend ahead!

P.S. I'm not sure if I should close the issue here - if so I'm happy to do so.
thomwolf (MEMBER) commented on 2020-06-20 20:51:27:
Thanks a lot, @aleSuglia for the very detailed and introductive feature request.
It seems like we could build something pretty useful here indeed.

One of the questions here is that Arrow doesn't have built-in support for generic "tensors" in records but there might be ways to do that in a clean way. We'll probably try to tackle this during the summer.
thomwolf (MEMBER) commented on 2020-06-20 21:22:25:
I think I would probably go for a `MultiDataset` wrapper around a list of `Dataset`.

I'm not sure we need to give it `k` and `t` parameters at creation, it can maybe be something along the lines of:
```python
squad = nlp.load_dataset("squad")
cnn_dm = nlp.load_dataset("cnn_dailymail","3.0.0")

multitask_dataset = nlp.MultiDataset(squad, cnn_dm)

batch = multitask_dataset.sample(10, temperature=2.0, k=1000)
```

The first proof-of-concept for multi-task datasets could definitely require that the provided datasets have the same name/type for columns (if needed you easily rename/cast a column prior to instantiating the `MultiDataset`).

It's good to think about it for some time though and don't overfit too much on the T5 examples (in particular for the ways/kwargs for sampling among datasets).
patrickvonplaten (CONTRIBUTOR) commented on 2020-06-22 08:33:32:
This is a super useful PR :-) @sshleifer - maybe you can take a look at the updated version of xsum if you can use it for your use case. Now, one should be able to just load it with:

```python 
nlp.load_datasets("xsum", ....)  # no manual dir required anymore
```
lbourdois (NONE) commented on 2020-06-22 11:39:24:
Hi

Any news about this PR ?
Because thinking back FLUE basically offers only two new datasets : those for the Word Sense Disambiguation task (vsd and nsd).

Wouldn't it be more clever to make separate PRs to add the datasets of the other tasks which are multi-lingual (and therefore can be used for other languages) ?

XNLI being already present on your library, there would only be PAWS-X (datasets and bibtex available here : https://github.com/google-research-datasets/paws/tree/master/pawsx) and the Webis-CLS-10 dataset (dataset : https://zenodo.org/record/3251672#.XvCXN-d8taQ and bibtex : https://zenodo.org/record/3251672/export/hx#.XvCXZ-d8taQ) to do.

And next for the FLUE benchmark, all you would have to do would be to use your own library by making an nlp.load_dataset() (for example nlp.load_dataset('xnli') which is already present in your library) for each of the datasets of the benchmark tasks and to keep only the 'fr' data.



Also @mariamabarham , did you get any feedback for the parsing task dataset request?
In case of refusal from the authors, there are other datasets in French to perform this task and in this case, I would open a new topic
mariamabarham (CONTRIBUTOR) commented on 2020-06-22 15:35:09:
Hi @lbourdois ,
PAWS-X is also present in the lib, it's part of `xtreme` dataset, so it can be loaded by `nlp.load_dataset('xtreme', 'PAWS-X.fr')` for the french version.
I think the parsing and the Word Sense Disambiguation task datasets are the only missing in the lib now. 
I did not get a feedback yet for the parsing dataset.
lucidrains (CONTRIBUTOR) commented on 2020-06-22 22:57:06:
Got around to this today, and so far so good, I'm able to download and load pg19 locally. However, I think there may be an issue with the dummy data, and testing in general.

The problem lies in the fact that each book from pg19 actually resides as its own text file in a google cloud folder that denotes the split, where the book id is the name of the text file. https://console.cloud.google.com/storage/browser/deepmind-gutenberg/train/ I don't believe there's anywhere else (even in the supplied metadata), where the mapping of id -> split can be found.

Therefore I end up making a network call `tf.io.gfile.listdir` to get all the files within each of the split directories. https://github.com/lucidrains/nlp/commit/adbacbd85decc80db2347d0882e7dab4faa6fd03#diff-cece8f166a85dd927caf574ba303d39bR78

Does this network call need to be eventually stubbed out for testing?
TheophileBlard (CONTRIBUTOR) commented on 2020-06-23 07:54:55:
> By the way, @TheophileBlard I commented some days ago in your fork. It would be great if you can maybe open a new PR with your code or if you have a better way to make it available to others for review.

Yea sorry, missed that! I think @lbourdois has a point, it helps no one to have the same dataset in multiple places. I will try to find some time to adapt the code of my fork and open PRs for `Webis-CLS-10` and `nsd`/`vsd`. Maybe we should group `nsd`/`vsd` together ?
lhoestq (MEMBER) commented on 2020-06-23 10:24:28:
Thanks for reporting these errors :)

I can actually see two issues here.

First, datasets like `natural_questions` require apache_beam to be processed. Right now the import is not at the right place so we have this error message. However, even the imports are fixed, the nlp viewer doesn't actually have the resources to process NQ right now so we'll have to wait until we have a version that we've already processed on our google storage (that's what we've done for wikipedia for example).

Second, datasets like `newsroom` require manual downloads as we're not allowed to redistribute the data ourselves (if I'm not wrong). An error message should be displayed saying that we're not allowed to show the dataset.

I can fix the first issue with the imports but for the second one I think we'll have to see with @srush to show a message for datasets that require manual downloads (it can be checked whether a dataset requires manual downloads if `dataset_builder_instance.manual_download_instructions is not None`).
ghomasHudson (CONTRIBUTOR) commented on 2020-06-23 12:28:37:
The problem with changing `k` and `t` per sampling is that you'd have to somehow remember which examples you'd already returned while re-weighting the remaining examples based on the new `k` and `t`values. It seems possible but complicated (I can't really see a reason why you'd want to change the weighting of datasets after you constructed the multidataset).

Wouldn't it be convenient if it implemented the dataset interface? Then if someone has code using a single nlp dataset, they can replace it with a multitask combination of more datasets without having to change other code. We would at least need to be able to pass it into a `DataLoader`.
JohnGiorgi (CONTRIBUTOR) commented on 2020-06-23 14:07:34:
I was able to track down the file causing the problem by adding the following to `scientific_papers.py` (starting at line 116):

```python
                from json import JSONDecodeError
                try:
                    d = json.loads(line)
                    summary = "\n".join(d["abstract_text"])
                except JSONDecodeError:
                    print(path, line)
```



For me it was at: `/Users/johngiorgi/.cache/huggingface/datasets/f87fd498c5003cbe253a2af422caa1e58f87a4fd74cb3e67350c635c8903b259/arxiv-dataset/train.txt` with `"article_id": "1407.3051"`.

Not really 100% sure at the moment, but it looks like this specific substring from `"article_text"` may be causing the problem?

```
"after the missing - mass scale adjustment , the validity of the corrections was tested in the @xmath85 productions at 1.69 gev/@xmath1 . in fig .  [", "fig : calibrations ] ( a ) , we show the missing - mass spectrum in the @xmath86 region in the @xmath87 reaction at 1.69 gev/@xmath1 . a fitting result with a lorentzian function for the @xmath86  ( dashed line ) and the three - body phas
```

perhaps because it appears to be truncated. I (think) I can recreate the problem by doing the following:

```python
import json

# A minimal example of the json file that causes the error
invalid_json = '{"article_id": "1407.3051", "article_text": ["the missing - mass resolution was obtained to be 2.8 @xmath3 0.1  mev/@xmath4  ( fwhm ) , which corresponds to the missing - mass resolution of 3.2  @xmath3  0.2  mev/@xmath4  ( fwhm ) at the @xmath6 cusp region in the @xmath0 reaction .", "this resolution is at least by a factor of 2 better than the previous measurement with the same reaction ( 3.2@xmath595.5 mev/@xmath4 in @xmath84 )  @xcite .", "after the missing - mass scale adjustment , the validity of the corrections was tested in the @xmath85 productions at 1.69 gev/@xmath1 . in fig .  [", "fig : calibrations ] ( a ) , we show the missing - mass spectrum in the @xmath86 region in the @xmath87 reaction at 1.69 gev/@xmath1 . a fitting result with a lorentzian function for the @xmath86  ( dashed line ) and the three - body phas'   
# The line of code from `scientific_papers.py` which appears to cause the error
json.loads(invalid_json)
```

This is as far as I get before I am stumped.
lhoestq (MEMBER) commented on 2020-06-24 08:34:19:
Alright I think I've checked all your comments, thanks :)

Moreover I just added a way to serialize faiss indexes.
This is important because for big datasets the index construction can take some time.

Examples:

```python
ds = nlp.load_dataset('crime_and_punish', split='train')
ds_with_embeddings = ds.map(lambda example: {'embeddings': embed(example['line']}))
ds_with_embeddings.add_faiss_index(column='embeddings')
# query
scores, retrieved_examples = ds_with_embeddings.get_nearest_examples('embeddings', embed('my new query'), k=10)
# save index
ds_with_embeddings.get_index('embeddings').save('my_index.faiss')
```

```python
ds = nlp.load_dataset('crime_and_punish', split='train')
# load index
faiss_index = nlp.search.FaissIndex.load('my_index.faiss')
ds.add_faiss_index('embeddings', faiss_index=faiss_index)
# query
scores, retrieved_examples = ds.get_nearest_examples('embeddings', embed('my new query'), k=10)
```

Let me know what you think
thomwolf (MEMBER) commented on 2020-06-24 09:21:03:
Nice!

Here are a few comments:

I think it would be good to separate (1) the name of the column we use for indexing and (2) the name of the index itself, at least in our head. As I understand it, once the index is created, the column we used to create it is irrelevant so the column name will only be relevant in the `add_faiss_index` and we should be able to supply a different index name, e.g. `my_faiss_index`. When we reload an index, we don't really care about the column that was used to create it, right? so it's maybe better to have an `index_name` (which default to the column name for a simple user experience but it can also be something else and this should be clear in our head when we define the API).

I'm wondering if we should not have a triple of methods for each retrieval engine: `add_xxx_index`, `save_xxx_index` and `load_xxx_index` when `xxx` can be `faiss` or `elasticsearch`. I'm not a fan of exposing `nlp.search.FaissIndex` unless you think there is a strong reason to have the user learn this abstraction.

Last but not least, I think we should already think about hosting index on our S3. I would maybe go for something like this: host the index serialized with the cached dataset on user-provided namespaces:
```python
wiki_indexed = load_dataset('thom/wiki_indexed_with_dpr_faiss')
```
lhoestq (MEMBER) commented on 2020-06-24 11:08:41:
To summarize:


```python
ds = nlp.load_dataset('crime_and_punish', split='train')
ds_with_embeddings = ds.map(lambda example: {'embeddings': embed(example['line']}))
ds_with_embeddings.add_faiss_index(column='embeddings')
# query
scores, retrieved_examples = ds_with_embeddings.get_nearest_examples('embeddings', embed('my new query'), k=10)
# save index
ds_with_embeddings.save_faiss_index('embeddings', 'my_index.faiss')
```

```python
ds = nlp.load_dataset('crime_and_punish', split='train')
# load index
ds.load_faiss_index('embeddings', 'my_index.faiss')
# query
scores, retrieved_examples = ds.get_nearest_examples('embeddings', embed('my new query'), k=10)
```
ghomasHudson (CONTRIBUTOR) commented on 2020-06-24 18:08:40:
Does it make sense to download the full stories? I remember attempting to implement this dataset a while ago and ended up with something like:
```python
    def _split_generators(self, dl_manager):
        """Returns SplitGenerators."""

        dl_dir = dl_manager.download_and_extract(_DOWNLOAD_URL)
        data_dir = os.path.join(dl_dir, "narrativeqa-master")

        urls = {"test":{}, "train": {},"valid":{}}
        with open(os.path.join(data_dir,"documents.csv")) as f_in:
            csv_reader = csv.reader(f_in)
            next(csv_reader) # discard header row
            for i,row in enumerate(csv_reader):
                if i > 1572:
                    break
                if row != []:
                    urls[row[1]][row[0]] = row[3]

        url_files = {}
        for key in urls.keys():
            url_files[key] = dl_manager.download_and_extract(urls[key])

        return [
            nlp.SplitGenerator(
                name=nlp.Split.TRAIN,
                gen_kwargs={
                    "data_dir":data_dir,
                    "split":"train",
                    "doc_id_to_path":url_files["train"]
                    }
            ),
        ....
```
It does end up cluttering your huggingface cache dir though.
ghomasHudson (CONTRIBUTOR) commented on 2020-06-24 18:23:27:
Also since there doesn't seem to be any meaning in the order of answer_1 and answer_2, it might make sense to combine them (see [squad.py](https://github.com/huggingface/nlp/blob/8b0ffc85e4e52ae1f18d31be99b6c70b82c991ca/datasets/squad/squad.py#L86-L88)):
```python
"answers": nlp.features.Sequence({
    "text": nlp.Value("string"),
    "tokenized": nlp.features.Sequence(nlp.Value("string"))
})
```
(the tokenized features should also probably be lists of strings not just strings - see [natural_questions.py](https://github.com/huggingface/nlp/blob/4cd34287300a1135ce7b22f6dd209ca305c71b3a/datasets/natural_questions/natural_questions.py#L83))

Again, this is a personal preference thing, but it might be useful to combine the document-related features:
```python
{
    "document": {
        "id": nlp.Value("string"),
        "kind": nlp.Value("string"),
        "url": nlp.Value("string"),
        "file_size": nlp.Value("int32"),
        "word_count": nlp.Value("int32"),
        "start": nlp.Value("string"),
        "end": nlp.Value("string"),
        "wiki_url": nlp.Value("string"),
        "wiki_title": nlp.Value("string"),
        "summary": nlp.features.Sequence({
            "text": nlp.Value("string"),
            "tokens": nlp.features.Sequence(nlp.Value("string"))
        }),
        "text": nlp.Value("string"),
    },
    "question": nlp.features.Sequence({
        "text": nlp.Value("string"),
        "tokens": nlp.features.Sequence(nlp.Value("string"))
    }),
    "answers": nlp.features.Sequence({
        "text": nlp.Value("string"),
        "tokens": nlp.features.Sequence(nlp.Value("string"))
    })
}
```
jarednielsen (CONTRIBUTOR) commented on 2020-06-24 23:21:17:
Thanks for the pointer to those functions! It's still a little more verbose since you have to manually calculate which ids each rank would keep, but definitely works.

My use case is multi-node, multi-GPU training and avoiding global batches of duplicate elements. I'm using horovod. You can shuffle indices, or set random seeds, but explicitly sharding the dataset up front is the safest and clearest way I've found to do so.
ghomasHudson (CONTRIBUTOR) commented on 2020-06-26 11:11:16:
Good spot! Here are my thoughts:

- Aside: Adding `MultitaskModel` to transformers might be a thing to raise - even though having task-specific heads has become unfashionable in recent times in favour of text-to-text type models.
- Adding the task name as an extra field also seems useful for these kind of models which have task-specific heads
- There is some validation of our approach that the user should be expected to `map` datasets into a common form.
- The size-proportional sampling (also called "Examples-proportional mixing") used here doesn't perform too badly in the T5 paper (it's comparable to temperature-scaled mixing in many cases but less flexible. This is only reasonable with a `K` maximum size parameter to prevent very large datasets dominating). This might be good for a first prototype using:
    ```python
    def __iter__(self):
        """
        For each batch, sample a task, and yield a batch from the respective
        task Dataloader.

        We use size-proportional sampling, but you could easily modify this
        to sample from some-other distribution.
        """
        task_choice_list = []
        for i, task_name in enumerate(self.task_name_list):
            task_choice_list += [i] * self.num_batches_dict[task_name]
        task_choice_list = np.array(task_choice_list)
        np.random.shuffle(task_choice_list)

        dataloader_iter_dict = {
            task_name: iter(dataloader) 
            for task_name, dataloader in self.dataloader_dict.items()
        }
        for task_choice in task_choice_list:
            task_name = self.task_name_list[task_choice]
            yield next(dataloader_iter_dict[task_name])    
    ```
    We'd just need to pull samples from the raw datasets and not from `DataLoader`s for each task. We can assume the user has done `dataset.shuffle()` if they want to.

    Other sampling methods can later be implemented by changing how the `task_choice_list` is generated. This should allow more flexibility and not tie us to specific methods for sampling among datasets.
AmitMY (CONTRIBUTOR) commented on 2020-06-26 16:17:38:
Even more complicating - 

As I see it, datasets can have "addons".
For example, the WebNLG dataset is a dataset for data-to-text. However, a work of mine and other works enriched this dataset with text plans / underlying text structures. In that case, I see a need to load the dataset "WebNLG" with "plans" addon.

Same for sign language - if there is a dataset of videos, one addon can be to run OpenPose, another to run ARKit4 pose estimation, and another to run PoseNet, or even just a video embedding addon. (which are expensive to run individually for everyone who wants to use these data)

This is something I dabbled with my own implementation to a [research datasets library](https://github.com/AmitMY/meta-scholar/) and I love to get the discussion going on these topics.
jarednielsen (CONTRIBUTOR) commented on 2020-06-26 23:35:08:
Update: I think I've found a solution.

```python
output_types = {"input_ids": tf.int64, "token_type_ids": tf.int64, "attention_mask": tf.int64}
def train_dataset_gen():
    for i in range(len(train_dataset)):
        yield train_dataset[i]
tf_dataset = tf.data.Dataset.from_generator(train_dataset_gen, output_types=output_types)
```

loads WikiText-2 in 20 ms, and WikiText-103 in 20 ms. It appears to be lazily loading via indexing train_dataset.
lhoestq (MEMBER) commented on 2020-06-28 21:02:10:
This is a really cool idea !
The example for data objects you gave for the RWTH-PHOENIX-Weather 2014 T dataset can totally fit inside the library.

For your point about formats like `ilex`, `eaf`, or `srt`, it is possible to use any library in your dataset script.
However most user probably won't need these libraries, as most datasets don't need them, and therefore it's unlikely that we will have them in the minimum requirements to use `nlp` (we want to keep it as light-weight as possible). If a user wants to load your dataset and doesn't have the libraries you need, an error is raised asking the user to install them.

More generally, we plan to have something like a `requirements.txt` per dataset. This could also be a place for addons as you said. What do you think ?
lhoestq (MEMBER) commented on 2020-06-28 21:16:48:
For one dataset you can have different configurations that each have their own `nlp.Features`.
We imagine having one configuration per subtask for example.
They are loaded with `nlp.load_dataset("my_dataset", "my_config")`.

For example the `glue` dataset has many configurations. It is a bit different from your case though because each configuration is a dataset by itself (sst2, mnli).
Another example is `wikipedia` that has one configuration per language.
lhoestq (MEMBER) commented on 2020-06-28 21:32:46:
Quick update on NQ: due to some limitations I met using apache beam + parquet I was not able to use the dataset in a nested parquet structure in python to convert it to our Apache Arrow format yet.
However we had planned to change this conversion step anyways so we'll make just sure that it enables to process and convert the NQ dataset to arrow.
lhoestq (MEMBER) commented on 2020-06-28 21:42:16:
We plan to have something like a `requirements.txt` per dataset to prevent user from loading dataset with old version of `nlp` or any other libraries. Right now the solution is just to keep `nlp` up to date when you want to load a dataset that leverages the latests features of `nlp`.

For datasets that are on AWS but that use features that are not released yet we should be able to filter those from the `list_dataset` as soon as we have the `requirements.txt` feature on (filter datasets that need a future version of `nlp`).

Shall we rename this issue to be more explicit about the problem ?
Something like `Specify the minimum version of the nlp library required for each dataset` ?
zphang (NONE) commented on 2020-06-29 08:18:33:
Hey! Happy to jump into the discussion here. I'm still getting familiar with bits of this code, but the reasons I sampled over data loaders rather than datasets is 1) ensuring that each sampled batch corresponds to only 1 task (in case of different inputs formats/downstream models) and 2) potentially having different batch sizes per task (e.g. some tasks have very long/short inputs). How are you currently dealing with these in your PR?
ghomasHudson (CONTRIBUTOR) commented on 2020-06-29 08:34:54:
The short answer is - I'm not! Everything is currently on a per-example basis. It would be fairly simple to add a `batch_size` argument which would ensure that every `batch_size` examples come from the same task. That should suit most use-cases (unless you wanted to ensure batches all came from the same task and apply something like `SortishSampler` on each task first)

Your notebook was really inspiring by the way - thanks!
lhoestq (MEMBER) commented on 2020-06-29 17:06:09:
 I think it's fine and I can't think of another way to make the import work anyways.

Maybe we can have the `sys.path` behavior inside `prepare_module` instead ? Currently it seems to come out of nowhere in the code ^^'
We could check if external imports have a `__init__.py` and if it is the case then we can add to directory to the `PYTHONPATH`