fixed bug in run_mlm_flax_stream.py

[KennethEnevoldsen]

What does this PR do?

Fixed bug caused by additional keys (id, text) referring to non-list values when concatenating samples. An alternative option is to drop these columns in the dataset before passing it to iter.

Fixes #17132 by @HLasse.

Before submitting

• This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
• Did you read the contributor guideline,
  Pull Request section?
• Was this discussed/approved via a Github issue or the forum? Please add a link
  to it if that's the case.
• Did you make sure to update the documentation with your changes? Here are the
  documentation guidelines, and
  here are tips on formatting docstrings.
• Did you write any new necessary tests?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@patrickvonplaten (tagged in bug)
@patil-suraj (assigned to bug)

[HuggingFaceDocBuilderDev]

The documentation is not available anymore as the PR was closed or merged.

[patil-suraj]

Thanks for the PR. This solution looks good, but I think the better option would to just drop the other columns, like its done in PT script.

transformers/examples/pytorch/language-modeling/run_mlm.py

Line 379 in a42242d

column_names = raw_datasets["train"].column_names

transformers/examples/pytorch/language-modeling/run_mlm.py

Line 440 in a42242d

remove_columns=column_names,

[KennethEnevoldsen]

No problem. Tried using the remove_columns argument as in the example but the map functions do not take that argument (probably easy to add):

tokenized_datasets = dataset.map(
        tokenize_function,
        batched=True,
        remove_columns=column_names,
    )

Similar it might be nice for consistency to add the .column_names extensions to the IterableDataset.

[patil-suraj]

No problem. Tried using the remove_columns argument as in the example but the map functions do not take that argument (probably easy to add):

tokenized_datasets = dataset.map(
        tokenize_function,
        batched=True,
        remove_columns=column_names,
    )

Similar it might be nice for consistency to add the .column_names extensions to the IterableDataset.

Pinging @lhoestq here. Is not possible to pass remove column to streaming dataset ?

[lhoestq]

remove_columns does exist:

https://huggingface.co/docs/datasets/v2.2.1/en/package_reference/main_classes#datasets.IterableDataset.map.remove_columns

[KennethEnevoldsen]

Indeed it does. I was using datasets version 1.17.0. Rerunning with an updated version indeed resolved the issue.

[KennethEnevoldsen]

Another note here, seems like the max_seq_length state that each text is truncated to e.g. max_seq_length, but from what I can read in the advance_iter_and_group_samples it seems like they are concatenated and split in groups of max_seq_length (with the remainder being discarded)

I would probably change it from:

    max_seq_length: Optional[int] = field(
        default=None,
        metadata={
            "help": "The maximum total input sequence length after tokenization. Sequences longer "
            "than this will be truncated. Default to the max input length of the model."
        },
    )

to:

    max_seq_length: Optional[int] = field(
        default=None,
        metadata={
            "help": "The maximum total input sequence length after tokenization. Sequences are concatenated in groups of max_seq_length. Default to the max input length of the model."
        },
    )

[patil-suraj]

LGTM!

[patil-suraj]

Another note here, seems like the max_seq_length state that each text is truncated to e.g. max_seq_length, but from what I can read in the advance_iter_and_group_samples it seems like they are concatenated and split in groups of max_seq_length (with the remainder being discarded)

I would probably change it from:

    max_seq_length: Optional[int] = field(
        default=None,
        metadata={
            "help": "The maximum total input sequence length after tokenization. Sequences longer "
            "than this will be truncated. Default to the max input length of the model."
        },
    )

to:

    max_seq_length: Optional[int] = field(
        default=None,
        metadata={
            "help": "The maximum total input sequence length after tokenization. Sequences are concatenated in groups of max_seq_length. Default to the max input length of the model."
        },
    )

Good catch! Feel free to open another PR if you would like to fix this.