🐛 fix matching algorithm, add logging and a spec helper. Fixes #73

[benprew]

Refactor matching algorithm (Cosine Similarity with TF-IDF) into separate class and fix
bug identified in issue #73.

[benprew]

@cantino Can you take a look at this when you get a chance? I refactored the tf-idf and cosine similarity calculations into a separate class.

Thanks

[cantino]

A little hard to follow since I'm no longer familiar with the code. Was this just a refactor (moving things around), or did you change logic too?

[benprew]

Yeah, it ended up being a bigger change than expected. The logic, however, is basically the same, my only changes were: 1. Previously IDF was calculated like this: log( (1 + N) / (1 + n_t) ) Where: N = number of accounts (documents) n_t = number of accounts with token t in them Which results in 0 if both N and n_t are 1, which is the case where you have a single account that you're trying to match to. I changed it to: log( N / (1 + n_t ) ) + 1 Which the Wikipedia page ( https://en.wikipedia.org/wiki/Tf%E2%80%93idf#Inverse_document_frequency_2) calls "IDF smooth" 2. Previously similarity was calculated as the dot product of the 2 vectors: Similarity = A . B Where A is the transaction we're comparing and B is an account we are comparing against. I changed to also divide by the magnitude of A time the magnitude of B Similarity = ( A . B ) / (||A|| ||B||) Which is the calculation the Wikipedia page ( https://en.wikipedia.org/wiki/Cosine_similarity) uses. This normalizes values to between 1 and -1. It wasn't clear what range the previous calculation would return. I also use the matrix library to calculate the dot product and magnitude. The other changes were primarily refactoring, including: * moving learn! to the top of the file, since it's called immediately after initialize * refactoring account tokens into a method (learn_from_account_tokens) * refactoring adding a regexp into a method (add_regexp) * adding a LOGGER constant whose level is changed to INFO if verbose is specified * moving each_row_backwards farther up the file, to follow the logical flow of app.rb * adding a spec helper fixture_path to prepend the data_fixtures path to a filename * changing some specs from "should" to new "expect" format (rpsec complains that "should" is deprecated) * adding a regression spec for issue #73 Thanks

…

On Wed, Feb 5, 2020 at 7:54 PM Andrew Cantino ***@***.***> wrote: A little hard to follow since I'm no longer familiar with the code. Was this just a refactor (moving things around), or did you change logic too? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#79?email_source=notifications&email_token=AAAJHV7S57WUHWPMRTPVDLTRBOCYZA5CNFSM4KQGZKHKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK522FY#issuecomment-582724887>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAJHV4SEHUH3GZDNJ4INEDRBOCYZANCNFSM4KQGZKHA> .

[cantino]

That all makes sense to me. If you've tested it, sounds fine with me. Nice work on cleaning up the cosine logic.