Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Swap out sift3 for swift4 common #133

Merged
merged 1 commit into from Oct 15, 2016
Merged

Conversation

thatguyintech
Copy link
Contributor

Fix #113

From Siderite's blog:

After implementing Sift4, I can now tell you that the simple version is slightly better than Sift3 for small maxOffset values like 5, but it gets better as the value increases. The common version is a bit more complex, but the error decreases with 33% and maintains a low error for large maxOffset values.

I pretty much just copied over the source. I took out the maxDistance for simplicity, and set a default maxOffset of 5 for consistency.

@thatguyintech
Copy link
Contributor Author

@derrickko any thoughts on this?

@derrickko
Copy link
Member

@alberthu16 looks awesome. Did you update the tests to make sure they pass?

@thatguyintech
Copy link
Contributor Author

Thanks @derrickko! Actually, I didn't have to update the tests to make sure they pass. None of them broke, so I'm assuming the functionality is consistent. Should I add some more domain checks? There are no tests for the actual edit distance algorithm currently.

@thatguyintech
Copy link
Contributor Author

@derrickko thoughts on adding tests or leaving as is?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants