-
Notifications
You must be signed in to change notification settings - Fork 16
Fix off by one issue in edge embedding table size #109
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
Fix off by one issue in edge embedding table size #109
Conversation
This also needs some cleanup before it's ready for review. |
Ready for review now as the underlying issue has been fixed. This would still benefit from test coverage, but some changes to the test infrastructure need to be made to pull in the appropriate block. Going to work on that in a future PR that I'll open up soon. |
Test infrastructure was simpler than I thought. The test is included in this patch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more comments, but overall this looks good.
Fixes #108