perf: score each column once on the whole file instead of averaging chunk scores - #270
Open
ThibaudDauce wants to merge 3 commits into
Open
perf: score each column once on the whole file instead of averaging chunk scores#270ThibaudDauce wants to merge 3 commits into
ThibaudDauce wants to merge 3 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
detection went from 46.5s to 107.6s on
joconde.csvbetween 0.12.0 and main#249 removed every way a format could be dropped before the end of the file (the early stop, the zeroing on a failed batch,
zero_if_too_low), soadresse(proportion 0.55),communeandcode_postalare tested against every value of every column now. On a profile of joconde,adresse._isalone is 57s over 4.2M calls.We can't just put it back: while the file is still being read, a format below its threshold can climb back up, that's what #249 fixed. So we read first and score once at the end, from the
col_valueswe already accumulate for the profile. The total is known by then, so giving up is exact: as soon as a format has failed on more than(1 - proportion) * totalvalues, it's out.Not the 46.5s of 0.12.0, but 0.12.0 stopped reading at 910 000 rows out of 992 809 and dropped formats on the first chunk alone, which is what gave the truncated
total_linesand the missedcommune/code_postalon ValeursFoncieres.Some scores change since they were an average of per-batch scores, unweighted by batch length (a 20 rows batch counted as much as a 100 000 one). Same divergences as the ones listed in #248, worth a look on your side.
test_col_val,_build_remaining_tests_per_col,_apply_proportion_thresholdsandzero_if_too_loware gone, they only existed to score as we go. #249 had no tests so I added three on the new behaviour.