[datasets] revert whitespace filtering and fix svhn reco #987
Conversation
felixdittrich92
added
module: datasets
|
@frgfm I think it would be the best to filter the labels directly .. the dataset is fix and we know what we need to exclude feels cleaner instead of any encode-decode try/except block 馃槄 wdyt ? |
Codecov Report
@@ Coverage Diff @@
## main #987 +/- ##
=======================================
Coverage 94.88% 94.88%
=======================================
Files 134 134
Lines 5556 5556
=======================================
Hits 5272 5272
Misses 284 284
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
felixdittrich92
changed the title
There was a problem hiding this comment.
Thanks! just added a few questions
And about removing white spaces early on, we can if it helps! I was just raising the point that depending on the dataset, that filter might seriously reduce the number of samples.
Either way we need to make a well-informed decision :)
| parser.add_argument('-b', '--batch_size', type=int, default=32, help='batch size for evaluation') | ||
| parser.add_argument('-b', '--batch_size', type=int, default=1, help='batch size for evaluation') |
There was a problem hiding this comment.
isn't it best to specify the batch size in the CI but leave a decent batch size for training?
There was a problem hiding this comment.
It's only eval ;) in this case we ensure while try block that no sample is skipped which can be read (does not contain whitespace and chars in vocab)
There was a problem hiding this comment.
CI uses a batch size of 32 to speedup so it's not relevant if samples are skipped for testing
| parser.add_argument('-b', '--batch_size', type=int, default=32, help='batch size for evaluation') | ||
| parser.add_argument('-b', '--batch_size', type=int, default=1, help='batch size for evaluation') |
About the whitespaces that was a bit to much focused on docTR behavior but we need to provide the whole dataset for example if a user only wants to use the integrated datasets maybe for a model which is able to recognize also whitespaces |
This PR:
Any feddback is welcome 馃