More informative error message for DataCollatorForSeq2Seq

[CakeCrusher]

What does this PR do?

I ran into an error related to an incorrect shape of inputs when using DataCollatorForSeq2Seq. I learned that it had to do with the BatchEncoding class. I did not find the error message particularly helpful as it does not mention anything about the input shape. Therefore I added the extra line on the error message to help guide anyone else who runs into this error.

Fixes #15505
@stas00

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?

[stas00]

@CakeCrusher, if it's programmable - won't it be better to actually validate the input shape explicitly, and assert if it's wrong - instead of piling up possible errors to an already long error message?

[CakeCrusher]

@stas00 agreed ill make a check for it

[CakeCrusher]

I'm not sure if there are more keys to take take into account for this issue

[stas00]

I think there was a miscommunication there.

In the OP you suggested that there could be an error of passing wrongly shaped inputs - at least that's how I understood it. And I proposed that perhaps it'd be better to check if inputs are misformatted and assert if that's the case. But your newly proposed change is something totally different. So I think I lost you here.

Perhaps you could show an example of wrongly shaped inputs and then the corrected one?

[HuggingFaceDocBuilderDev]

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

[CakeCrusher]

Not sure if this feature of fast tokenizers applies for the other keys.

[CakeCrusher]

Hi @stas00 , is the pr ok?

[stas00]

This PR is waiting for your answer here: #17447 (comment)

[stas00]

@CakeCrusher, I think we lost each other here. Should we finish this PR?

[CakeCrusher]

@sgugger @stas00

Hi @stas00 sorry for the discontinuity, I am now able to focus and see this issue through.

Here is an example demonstrating successful and erring inputs:
https://colab.research.google.com/drive/16aLu6QrDSV_aUYRdpufl5E4iS08qkUGj?usp=sharing

I then made the following changes to overcome excessive nesting (a list containing a single item):
CakeCrusher/transformers@main...lead_nesting_solution

I understand the changes are pretty fundamental, but they work. I have yet to add the assert statement, since the nesting fix does the job forcefully. I was hoping to do an overarching PR, involving the new error message (or assert) and the fix (possibly parametrized so that it is not forced). What are your thoughts?

[stas00]

This is an interesting idea, but I'm concerned it might be (1) not backward compatible (2) I think it's best for the user to apply this function themselves. Perhaps if it's a useful util function we can provide it and assert with a message to use it instead?

And to remind my initial suggestion was:

• check if shape is wrong and raise a specific assert if it is wrong (with possible hints at how to fix it)

e.g. the inputs shape is wrong, expecting a, but got b....

won't that be a clean solution?

we can then discuss with others if they feel your proposed util function would be a good match to add.

[CakeCrusher]

@stas00

Perhaps if it's a useful util function we can provide it and assert with a message to use it instead?

That is an excellent idea.

I will have it ready early next week with a test.

Do you recommend I make a new PR for it or merge it to this one?

[CakeCrusher]

Hi @stas00 , I submitted a new PR for the aforementioned fixes. I have yet to add the test and proper docs. As for what I have so far please let me know what you think.

(My git tree was a mess, so that was largely why it's a new PR sorry about that.)

[stas00]

Apologies for taking a long time to follow up, @CakeCrusher

As I suggested in the first place I think your suggestion to assert on invalid input nesting is great.

I see you tried to move the helper util to datasets and it's not being welcomed there, as it's really a user's responsibility to prepare the data correctly.

Perhaps we just stick to the assert part and trust the user to figure out how to fix it?

@sgugger, are you ok with the assertion part of this PR on the deeply nested input? I'd guess that you too might be against the 2nd part of adding a helper util to remove excessive nesting as it's not generic enough.

[CakeCrusher]

No worries @stas00,
Yeah.. I understand if I have to give up on introducing the helper function on this PR. I'll see what what lhoestq ends up thinking about the datasets alternative.

In the meantime, I'll keep the assert independent. And maybe open a new PR for the helper function.

[sgugger]

I must admit I do not understand what the problem is, since the notebook linked executes without any issue.

[CakeCrusher]

Sorry about that @sgugger the notebook was organized in a weird way. Now the notebook will raise the error.

[sgugger]

I see. I've pointed out in #18119 where that error message should be updated.