improve std bench and add significant-digits

[maxim-uvarov]

Hi! I propose several changes to std bench:

• Remove insignificant precision by default (can be reverted with --sign-digits=0).
• Remove the 'times' field by default (can be returned with --list-timings).
• Add an option to set time units (inactive by default).

For removing insignificant precision, I needed a new command significant-digits which is also included in this PR. This command has tests.

[maxim-uvarov]

I will add tests to bench itself too

[maxim-uvarov]

Dear maintainers, please, take a look. I think it is ready for review

[fdncred]

I like it. Thanks!

[amtoine]

@maxim-uvarov 👋

i have a question: when --units is set, what is the type of the durations in the output? it looks like a string with ms at the end in your example, rigth?

[amtoine]

generally, i would say that all these changes are nice improvements but pretty much out of the scope of std bench 😕

std bench is just for measuring, formatting could be done in another command, e.g.

bench { sleep 0.1sec; 1 + 2 } --rounds 5 --verbose # the current `std bench` command
    | reject times                                 # same as not using `--list-timings` in your implementation
    | bench format --units ms --sign-digits 2      # same as your `--units` and `--sign-digits`

of course, that's my way of thinking, i like commands that do one thing 😌

[maxim-uvarov]

@amtoine, I understand your concern! I really envisioned this and spent a number of hours polishing my proposal before submitting it. My motivation was to create a more convenient function with better defaults.

My motivation comes from the perspective of a non-developer, for whom typing every symbol is much more effortful than for a seasoned professional like you (who might be the target audience for std bench).

Maybe you would agree with me that this representation is very wordy and unnecessary, even for a professional?

1sec 333ms 333µs 333ns

And times are expaneded by default with my settings.

Perhaps we can:

1. Incorporate the feature of truncating resetting digits after the fourth one for results (maximum error 0.05%, plus a duration stays as a duration).
2. Get rid of the times field from std bench since it is quite easy to produce times for further operations with

1..50 | each {timeit{something}}

and not introduce any new options and flags to std bench?

@amtoine What do you think?

[maxim-uvarov]

i have a question: when --units is set, what is the type of the durations in the output? it looks like a string with ms at the end in your example, rigth?

--units is an option. If it is unset - all the values are produced as duration. If it is set to some unit - it will produce formatted strings accordingly.

[amtoine]

@amtoine, I understand your concern! I really envisioned this and spent a number of hours polishing my proposal before submitting it. My motivation was to create a more convenient function with better defaults.

My motivation comes from the perspective of a non-developer, for whom typing every symbol is much more effortful than for a seasoned professional like you (who might be the target audience for std bench).

yeah, the target audience of std bench is people who would want to run benchmarks in Nushell, i.e. very likely seasoned professionals 😉

Maybe you would agree with me that this representation is very wordy and unnecessary, even for a professional?

1sec 333ms 333µs 333ns

maybe, but then i would argue the issue comes from the formatting nushell does on durations, not from std bench, it's just that Nushell will show every part of the duration 😕

And times are expaneded by default with my settings.

again, that's not the fault of std bench, but rather probably your display_output hook 😉
the intent of returning $.times is to allow the developer to perform other operations on the raw measurements without blindly trusting the $.mean and $.std! (seed nushell/nushell#12913)

Perhaps we can:

1. Incorporate the feature of truncating resetting digits after the fourth one for results (maximum error 0.05%, plus a duration stays as a duration).
2. Get rid of the times field from std bench since it is quite easy to produce times for further operations with

1..50 | each {timeit{something}}

and not introduce any new options and flags to std bench?

@amtoine What do you think?

1. maybe, i dunno 🤣 i think users that really want to run std bench in the REPL and not in a script could just throw format duration ms on the times and that would solve evertything without changing anything
2. i wouldn't because returning the raw times was one of the intent of std bench and i would argue it's not a huge burden for the users to add reject times 😋

again, these are just my two cents, i would not die on that, it's not critical 😌

[maxim-uvarov]

@amtoine thank you for explaining.

I just leave my opinion here without intention of convincing you. I won't die either if my proposal won't be accepted, no worries :)

1. maybe, i dunno 🤣 i think users that really want to run std bench in the REPL and not in a script could just throw format duration ms on the times and that would solve evertything without changing anything

AFAIK there is no easy way to format durations in the all record fields now. update cells works only with tables.

> bench -n 10 {sleep 0.1sec} | format duration ms
Error: nu::shell::only_supports_this_input_type

2. i wouldn't because returning the raw times was one of the intent of std bench and i would argue it's not a huge burden for the users to add reject times 😋

I have intuition, that users will need to write reject times much more often than to use bench without it. And when they need it - it is really easy to type.

1..50 | each {timeit{something}}

[amtoine]

@amtoine thank you for explaining.

🙏

I just leave my opinion here without intention of convincing you. I won't die either if my proposal won't be accepted, no worries :)

❤️

AFAIK there is no easy way to format durations in the all record fields now. update cells works only with tables.

> bench -n 10 {sleep 0.1sec} | format duration ms
Error: nu::shell::only_supports_this_input_type

i took out my wizard wand and found a solution to this 😉

bench -n 10 {sleep 0.1sec} | reject times | format duration ms ...($in | columns)

I have intuition, that users will need to write reject times much more often than to use bench without it. And when they need it - it is really easy to type.

1..50 | each {timeit{something}}

true, but that last command is precisely what std bench has been written for, to avoid writing that each time, but bench { something } --rounds 50 instead 😏

[maxim-uvarov]

bench -n 10 {sleep 0.1sec} | reject times | format duration ms ...($in | columns)

that is cool! thank you, @amtoine! 🙏

--units is really unnecessary then