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

Use units based on powers of 2 #1583

Merged
merged 2 commits into from
Jul 25, 2024
Merged

Conversation

e10e3
Copy link
Contributor

@e10e3 e10e3 commented Jul 25, 2024

Where unit multiples of the byte are used, this PR uses the unambiguous name of the unit.

Binary units have a different symbol (KiB, MiB) to distinguish them from the general-use decimal units (kB, MB). River only uses the binary units.

The only functional change is in tree.utils.calculate_object_size, where the recognised valued for the second (optional) argument were changed. This optional argument was not used in River.

Closes #1582

This reflects how they are interpreted in the code.
@MaxHalford
Copy link
Member

Thanks, this is a cool kind of cleanup. Thanks a lot for adding an entry to the release docs!

@MaxHalford MaxHalford merged commit 0ce2bec into online-ml:main Jul 25, 2024
4 checks passed
@smastelini
Copy link
Member

smastelini commented Jul 25, 2024

Oh, I didn't see this PR. I was working in parallel on another PR that actually used MB, GB, and so on. For completeness, as we opted for the binary units, the guideline on Hoeffding trees should also be updated! Thanks for the PR @e10e3!

@smastelini
Copy link
Member

smastelini commented Jul 25, 2024

I will take care of the tree guidelines!

Edit: done!

@e10e3 e10e3 deleted the binary-size-units branch July 25, 2024 17:04
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.

The wrong units are used for binary size
3 participants