-
Notifications
You must be signed in to change notification settings - Fork 12
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
use binary strings in dictionaries to save memory #128
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #128 +/- ##
==========================================
+ Coverage 96.34% 96.38% +0.03%
==========================================
Files 33 33
Lines 575 581 +6
==========================================
+ Hits 554 560 +6
Misses 21 21 ☔ View full report in Codecov by Sentry. |
@juanjoDiaz I believe the PR is ready to be merged, do you have anything to add? Edit: all the tests pass and the evaluation results are strictly the same on my computer. |
Sorry for the slow response. Did you test the performance impact of this? |
Yes, the difference in performance is either not noticeable or not significant. The package is a little bit lighter even with compressed pickle files so this is obviously good. |
See discussions in #72 and #125.
The PR cannot be merged until the dictionaries have been re-processed to account for the change.