Consistency with sorting: torch.RNN default vs torchtext.Iterator default about text HOT 4 CLOSED

I've thought about this a bit, and I used to think that sorting should be part of the data code. Now, I'm not so sure. Say you're doing inference --- you need your batches to be in sorted order, but they don't come in sorted order. If you sort within a batch using torchtext, it seems excessive for torchtext to provide an API for unshuffling your batch predictions (since what good are predictions if they're in random order wrt the input?)

Long story short, I sort the batches in my code, vs relying on torchtext to do it. It's pretty speedy so I haven't had a reason to complain (yet)

Regardless, I wouldn't call this a bug, more a design decision...

from text.

@nelson-liu Not sure we're on the same page.

Do not think I am asking for: "torchtext to provide an API for unshuffling your batch predictions"
Not asking for this either "sort the batches".

Specifically, I am referencing sorting the sequences within a batch by decreasing lengths for both inference and training. For example:

Batch[
   "1 2 3"
   "1 2 3 4 5"
   "1 2 3 4"
]

Will throw an error with torch.RNN and pack_padded_sequence unless the batch is sorted by decreasing lengths:

Batch[
   "1 2 3 4 5"
   "1 2 3 4"
   "1 2 3"
]

from text.

It's always possible for the user to provide a sort_key that's the negative of what they would otherwise provide. Would that be enough to make the default behavior consistent with the RNN API? Should we default to treating sort_keys as implicitly negative? Or provide an additional kwarg for Iterators to reverse their sort order?

from text.

@jekbradbury I think the sort_key should remain as it is. Otherwise, it would not be consistent with default sorting behavior.

Maybe add a parameter reverse and have it defaulted to True. That would be consistent with sort and torch.RNN. Like you mentioned!

On another note:
Going to close this issue. Unable to replicate my original bug.

from text.