Skip to content

fix(formatting): improve accuracy#178

Merged
imsnif merged 4 commits intomainfrom
accuracy
Sep 2, 2020
Merged

fix(formatting): improve accuracy#178
imsnif merged 4 commits intomainfrom
accuracy

Conversation

@imsnif
Copy link
Copy Markdown
Owner

@imsnif imsnif commented Sep 1, 2020

This fix improves the accuracy presentation by dividing the bytes per second by the right divisions (eg. 1_048_576 instead of 1_000_000 for MB).

@imsnif imsnif requested a review from TheLostLambda September 1, 2020 07:20
@TheLostLambda
Copy link
Copy Markdown
Collaborator

This looks good to me and is probably more true to how most bandwidth is recorded, but we should probably change to the proper units as well (eg "MB" -> "MiB").

As I understand, things like MB and KB are the SI units divided just by things like 1_000_000 where the binary versions like 1_048_576 are Mebibytes as opposed to Megabytes officially. They end up being mixed up a lot I think, but it wouldn't hurt to add the extra "i" to be clear. So things like "KiB", "MiB", "GiB", and "TiB".

Otherwise, looks like a good change! It could be extra work, but we could also have a flag for picking the units? The only reason I say that is it may be confusing if you download a 1GB file and see something like 0.93GiB in the cumulative mode

@imsnif
Copy link
Copy Markdown
Owner Author

imsnif commented Sep 2, 2020

That's interesting. Hum... FF and wget seem to be reporting these as {M,K}B/s. Running some manual tests, it seems to me* that we match both of these. So do you think this is a "mistake" on their part, or just an omission to make things simple?

*hard to be 100% sure because we're averaging our reporting over 3 seconds

@imsnif
Copy link
Copy Markdown
Owner Author

imsnif commented Sep 2, 2020

Did some tests with -t and we're really dead-on with what wget reports. So I guess they're reporting mebibytes too. I'll change everything to {mebi}bytes. Learned something new - thanks! :)

I dunno about the flag. I want to try to avoid adding more flags tbh. Maybe if someone specifically needs the distinction?

@TheLostLambda
Copy link
Copy Markdown
Collaborator

Haha, upon looking into this further, it seems like a bit of a mess. From what I can gather, FF uses the powers-of-two even when the operating system (like Linux and MacOS, I believe) use the SI powers-of-ten convention. Here is a bug report about that and an ancient, still open one on a similar topic.

I think wget does the same?

I think though, that this is the standard the Unixes have settled on for the most part?

And yeah, on second thought, it's probably not worth the flag. I think that moving to the "ibi" units is nice just to be totally unambiguous :)

@imsnif
Copy link
Copy Markdown
Owner Author

imsnif commented Sep 2, 2020

Alright, changed everything to ibis. :) Going to merge this. Thanks for the valuable input!

@imsnif imsnif merged commit dbc73a4 into main Sep 2, 2020
@imsnif imsnif deleted the accuracy branch September 13, 2020 16:27
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.

2 participants