Skip to content

Commit

Permalink
simplify the std bench improvement candidate (#865)
Browse files Browse the repository at this point in the history
@amtoine gave me valuable feedback about the [PR of `std bench`
improvement](#859) CANDIDATE
into `stdlib-candidates`.

I understood and respected his reasoning about his position. Yet, I
still believe that users of `std bench` will benefit from my proposal.

I incorporated some of @amtoine advice in this PR.

I removed the `bench --units` option as I now believe it is better to
encourage users to use core functionality for formatting duration (which
I had not thought of initially) and to avoid multiplying `--options`.

```
bench -n 10 {sleep 0.1sec} | format duration ms mean min max stddev
# or
bench -n 10 {sleep 0.1sec} | format duration ms ...($in | columns)
```

Also, I removed the option of `--sign-digits` and just hard-coded the
precision of conversion to the fourth significant digit (which will give
a maximum relative error of 0.05%, which I still think is unnecessarily
precise).

To explain my motivation, I added some context from our previous
conversation:

>> 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 I add here that if Nushell adds the setting for resetting
insignificant digits from displaying, those conversions could be removed
for the better. Yet, we don't have such a setting yet, but we already
use bench and use it quite often. So, I propose this usability
improvement.

In my defense, I would add that the existing `--pretty` option will only
benefit from the proposed changes (while it can't benefit from `| format
duration ms`).

```diff
> bench {sleep (10sec / 3)} --rounds 2 --pretty
- 3sec 335ms 974µs 264ns +/- 1ms 108µs 748ns
+ 3sec 335ms +/- 1ms 108µs
```

Finally, I kept the `--list-timings` option because I strongly believe
that users much more often will not need expanded 50 lines of timing
results on their screen (which they can get rid of by adding `| reject
time` in the second execution of the `bench` command - but I would like
to avoid this second execution).

I won't be hurt if my proposed changes aren't accepted and applied to
mainstream. Yet, I feel like my initial PR is still valuable and will
benefit from my current PR's additions.
  • Loading branch information
maxim-uvarov committed Jun 3, 2024
1 parent f41d050 commit 308d858
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 33 deletions.
30 changes: 7 additions & 23 deletions stdlib-candidate/std-rfc/bench.nu
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,13 @@ use ./math

# convert an integer amount of nanoseconds to a real duration
def "from ns" [
--units: string # units to convert duration to (min, sec, ms, µs, ns)
--sign-digits: int # a number of first non-zero digits to keep (default 4; set 0 to disable rounding)
--sign-digits: int = 4 # a number of first non-zero digits to keep (default 4; set 0 to disable rounding)
] {
if $sign_digits == 0 {} else {
math significant-digits $sign_digits
}
| $"($in)ns"
| into duration
| if $units != null {
format duration $units
} else {}
}

# run a piece of `nushell` code multiple times and measure the time of execution.
Expand All @@ -33,7 +29,6 @@ def "from ns" [
#
# > **Note**
# > `std bench --pretty` will return a `string`.
# > the `--units` option will convert all durations to strings
#
# # Examples
# measure the performance of simple addition
Expand All @@ -46,18 +41,9 @@ def "from ns" [
# ╰──────┴───────────╯
#
# get a pretty benchmark report
# > std bench {1 + 2} --pretty
# > bench {1 + 2} --pretty
# 922ns +/- 2µs 40ns
#
# format results as `ms`
# > bench {sleep 0.1sec; 1 + 2} --units ms --rounds 5
# ╭──────┬───────────╮
# │ mean │ 104.90 ms │
# │ min │ 103.60 ms │
# │ max │ 105.90 ms │
# │ std │ 0.75 ms │
# ╰──────┴───────────╯
#
# measure the performance of simple addition with 1ms delay and output each timing
# > bench {sleep 1ms; 1 + 2} --rounds 2 --list-timings | table -e
# ╭───────┬─────────────────────╮
Expand All @@ -75,9 +61,7 @@ export def main [
--rounds (-n): int = 50 # the number of benchmark rounds (hopefully the more rounds the less variance)
--verbose (-v) # be more verbose (namely prints the progress)
--pretty # shows the results in human-readable format: "<mean> +/- <stddev>"
--units: string # units to convert duration to (min, sec, ms, µs, ns)
--list-timings # list all rounds' timings in a `times` field
--sign-digits: int = 4 # a number of first non-zero digits to keep (default 4; set 0 to disable rounding)
] {
let times = seq 1 $rounds
| each {|i|
Expand All @@ -88,16 +72,16 @@ export def main [
if $verbose { print $"($rounds) / ($rounds)" }

{
mean: ($times | math avg | from ns --units $units --sign-digits=$sign_digits)
min: ($times | math min | from ns --units $units --sign-digits=$sign_digits)
max: ($times | math max | from ns --units $units --sign-digits=$sign_digits)
std: ($times | math stddev | from ns --units $units --sign-digits=$sign_digits)
mean: ($times | math avg | from ns --sign-digits 4)
min: ($times | math min | from ns --sign-digits 4)
max: ($times | math max | from ns --sign-digits 4)
std: ($times | math stddev | from ns --sign-digits 4)
}
| if $pretty {
$"($in.mean) +/- ($in.std)"
} else {
if $list_timings {
merge { times: ($times | each { from ns --units $units --sign-digits 0 }) }
merge { times: ($times | each { from ns --sign-digits 0 }) }
} else {}
}
}
10 changes: 0 additions & 10 deletions stdlib-candidate/tests/bench.nu
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
use std assert
use ../std-rfc bench

export def "test bench-units" [] {
let $test = bench {sleep 0.001sec; 1 + 2} --units ns --rounds 2 | get mean
assert str contains $test " ns"
}

export def "test bench-timings" [] {
let $test = bench {1 + 2} --rounds 3 --list-timings | get times | length
assert equal $test 3
Expand All @@ -15,8 +10,3 @@ export def "test bench-pretty" [] {
let $test = (bench {1 + 2} --rounds 3 --pretty) =~ '\d.* \+/- \d'
assert equal $test true
}

export def "test bench-sign-digits" [] {
let $test = bench {sleep 1ms} --sign-digits 1 --rounds 5 | get min
assert equal $test 1ms
}

0 comments on commit 308d858

Please sign in to comment.