-
Notifications
You must be signed in to change notification settings - Fork 1
chore: update Gonum fork and require Go 1.27.1 #171
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
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
54d5d6b
chore: upgrade Gonum fork to v0.19.0-fork
jamestjsp 17753c3
chore: address Gonum upgrade review with local pin and benchmarks
jamestjsp 2027e5d
chore: validate Gonum upgrade with Go 1.27.1
jamestjsp e290fe4
docs: retain full Go 1.27 Gonum performance comparison
jamestjsp File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| # Gonum v0.19.0-fork upgrade: performance evidence | ||
|
|
||
| For the updated PR requiring Go 1.27.1, see the [full follow-up](go1.27.1-full/README.md). | ||
| It substantially reduces the large regressions below, but confirms residual | ||
| slowdowns up to 6.17%. This original Go 1.26.4 report remains historical evidence. | ||
|
|
||
| Measured on 2026-09-12 for PR #171. The upgrade has mixed performance on this | ||
| machine. The complete 236-case timing geomean improves 0.75%, but reproducible | ||
| slowdowns remain. This is not a regression-free performance approval. Assess the | ||
| tradeoff or repair the affected backend paths before merging and publishing the | ||
| prepared v1.5.1 release. | ||
|
|
||
| ## Comparison | ||
|
|
||
| - Baseline: `fdda4f150d13b34a969de7faed6ae896e827ce94`, Gonum `v0.17.7-fork`. | ||
| - Candidate: `54d5d6b119c8b4351db0cd0bc0ebb4122bde94d4`, Gonum `v0.19.0-fork`. | ||
| - The baseline was the exact PR parent and freshly fetched `origin/main`. | ||
| - Apple M1 Pro, darwin/arm64, Go 1.26.4, `GOMAXPROCS=8`. | ||
| - Default Gonum Go BLAS and LAPACK implementations; no backend overrides, | ||
| custom build tags, GOFLAGS, or PGO profiles. Native compilation, no emulation. | ||
| - Same unchanged benchmark fixtures on both revisions. Both test binaries were | ||
| built before timing. No agent builds, tests, or profiling ran concurrently. | ||
| This was a normal desktop host, without CPU affinity or power-state control. | ||
| - Ten samples per case per revision, alternating baseline/candidate and | ||
| candidate/baseline order. Full suite: 100ms per benchmark; focused checks: | ||
| 300ms. All benchmark processes exited successfully with matching case sets. | ||
| - `benchstat`: `golang.org/x/perf v0.0.0-20260312031701-16a31bc5fbd0`. | ||
|
|
||
| ## Results | ||
|
|
||
| The longer follow-up covered the 14 full-sweep slowdowns of at least 3%, plus | ||
| DC motor simulation, regulator construction, and Lyapunov allocation behavior. | ||
| One nested benchmark needed a separate slash-separated selection. Medians below | ||
| come from the longer samples; all listed changes have p < 0.001, except the | ||
| 10,000-point SISO FRD case (p=0.009). Full tables include confidence intervals. | ||
|
|
||
| | Benchmark | Baseline | Candidate | Time change | | ||
| | --- | ---: | ---: | ---: | | ||
| | MatLog_N50 | 330.3 us | 372.0 us | +12.62% | | ||
| | D2C_ZOH_N50 | 413.3 us | 454.3 us | +9.92% | | ||
| | Stabsep_N100 | 8.904 ms | 9.455 ms | +6.18% | | ||
| | SystemFRD_SISO_10000 | 220.2 us | 233.7 us | +6.15% | | ||
| | Reduce | 45.94 us | 48.43 us | +5.42% | | ||
| | Modsep_N50 | 1.429 ms | 1.504 ms | +5.29% | | ||
| | Stabsep_N50 | 1.385 ms | 1.458 ms | +5.27% | | ||
| | DiscretizeZOH | 21.48 us | 22.58 us | +5.15% | | ||
| | MatLog_N20 | 49.37 us | 51.78 us | +4.88% | | ||
| | ControllabilityStaircase | 36.36 us | 37.94 us | +4.36% | | ||
| | D2C_ZOH_N20 | 61.47 us | 63.94 us | +4.02% | | ||
| | Canon_Modal_N50 | 640.5 us | 660.9 us | +3.19% | | ||
| | Stabsep_N10 | 32.70 us | 33.47 us | +2.37% | | ||
| | Simulate_DCMotor | 46.69 us | 39.72 us | -14.93% | | ||
| | Reg_N100_M5_P5 | 79.72 us | 60.92 us | -23.58% | | ||
|
|
||
| The `FrequencySweepKernels/N4_W2/TransferFunction` slowdown did not reproduce | ||
| with longer samples (p=0.447). Lyap_N100 timing was inconclusive (p=0.353). | ||
| Absence of statistical significance does not establish equivalence. The full | ||
| sweep contains additional smaller changes; see its complete table rather than | ||
| using this selected follow-up as an aggregate performance score. Multiple case | ||
| comparisons can produce false positives, especially for small effects. | ||
|
|
||
| Allocations also vary: DC motor simulation drops from 45 to 11 allocs/op and | ||
| regulator construction from 55 to 25. The focused SISO FRD case rises from 60 to | ||
| 61, and Stabsep_N100 from 481 to 482. Lyap_N100's full-sweep increase from 363 to | ||
| 364 did not reproduce in the focused run (both medians 363), although its bytes | ||
| per operation increased 0.08%. Tables retain all memory measurements. | ||
|
|
||
| These data establish behavior only on this host at eight workers. They do not | ||
| establish performance on amd64, other worker counts, or arbitrary models, and do | ||
| not attribute a specific backend change as the cause. Correctness tests and race | ||
| tests passed separately; performance measurements do not replace those tests. | ||
|
|
||
| ## Evidence and reproduction | ||
|
|
||
| - `full-benchstat.txt`: all 236 cases, timing and allocations. | ||
| - `focused-benchstat.txt`: 16 longer follow-up cases. | ||
| - `subcase-benchstat.txt`: the nested transfer-function follow-up. | ||
| - Matching `*-baseline.txt.gz` and `*-candidate.txt.gz`: complete raw samples. | ||
| - `metadata.json`: full-sweep binary hashes, flags, environment, and run order. | ||
|
|
||
| From the repository, build isolated copies of the exact revisions, then run the | ||
| existing Go benchmarks. Prebuilding is equivalent to `go test -run '^$' | ||
| -bench=. -benchmem -benchtime=100ms -count=1`, with compilation outside timing. | ||
|
|
||
| ```bash | ||
| set -eu | ||
| bench_dir=$(mktemp -d) | ||
| mkdir "$bench_dir/baseline" "$bench_dir/candidate" | ||
| git archive fdda4f150d13b34a969de7faed6ae896e827ce94 | tar -x -C "$bench_dir/baseline" | ||
| git archive 54d5d6b119c8b4351db0cd0bc0ebb4122bde94d4 | tar -x -C "$bench_dir/candidate" | ||
| (cd "$bench_dir/baseline" && go test -c -o "$bench_dir/baseline.test") | ||
| (cd "$bench_dir/candidate" && go test -c -o "$bench_dir/candidate.test") | ||
| export GOMAXPROCS=8 | ||
| for round in $(seq 1 10); do | ||
| order='baseline candidate' | ||
| if [ $((round % 2)) -eq 0 ]; then order='candidate baseline'; fi | ||
| for label in $order; do | ||
| "$bench_dir/$label.test" -test.run='^$' -test.bench=. \ | ||
| -test.benchmem -test.benchtime=100ms -test.count=1 \ | ||
| >> "$bench_dir/$label.txt" | ||
| done | ||
| done | ||
| benchstat "$bench_dir/baseline.txt" "$bench_dir/candidate.txt" | ||
| ``` | ||
|
|
||
| For focused reproduction, use 300ms and select the top-level names in | ||
| `focused-benchstat.txt` with `^Benchmark(Name1|Name2|...)$`. For the nested case, | ||
| use `^BenchmarkFrequencySweepKernels$/^N4_W2$/^TransferFunction$` separately. | ||
| To reanalyze retained data, decompress each pair with `gzip -dc` and pass the | ||
| resulting files to the recorded benchstat version. |
Binary file not shown.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| goos: darwin | ||
| goarch: arm64 | ||
| pkg: github.com/jamestjsp/controlsys | ||
| cpu: Apple M1 Pro | ||
| │ baseline.txt │ candidate.txt │ | ||
| │ sec/op │ sec/op vs base │ | ||
| Reduce-8 45.94µ ± 1% 48.43µ ± 7% +5.42% (p=0.000 n=10) | ||
| DiscretizeZOH-8 21.48µ ± 1% 22.58µ ± 6% +5.15% (p=0.000 n=10) | ||
| ControllabilityStaircase-8 36.36µ ± 2% 37.94µ ± 3% +4.36% (p=0.000 n=10) | ||
| Simulate_DCMotor-8 46.69µ ± 1% 39.72µ ± 2% -14.93% (p=0.000 n=10) | ||
| Lyap_N100-8 4.052m ± 1% 4.048m ± 0% ~ (p=0.353 n=10) | ||
| Reg_N100_M5_P5-8 79.72µ ± 7% 60.92µ ± 2% -23.58% (p=0.000 n=10) | ||
| D2C_ZOH_N20-8 61.47µ ± 1% 63.94µ ± 1% +4.02% (p=0.000 n=10) | ||
| D2C_ZOH_N50-8 413.3µ ± 1% 454.3µ ± 1% +9.92% (p=0.000 n=10) | ||
| MatLog_N20-8 49.37µ ± 1% 51.78µ ± 1% +4.88% (p=0.000 n=10) | ||
| MatLog_N50-8 330.3µ ± 1% 372.0µ ± 3% +12.62% (p=0.000 n=10) | ||
| SystemFRD_SISO_10000-8 220.2µ ± 6% 233.7µ ± 3% +6.15% (p=0.009 n=10) | ||
| Canon_Modal_N50-8 640.5µ ± 1% 660.9µ ± 2% +3.19% (p=0.000 n=10) | ||
| Stabsep_N10-8 32.70µ ± 1% 33.47µ ± 1% +2.37% (p=0.000 n=10) | ||
| Stabsep_N50-8 1.385m ± 1% 1.458m ± 3% +5.27% (p=0.000 n=10) | ||
| Stabsep_N100-8 8.904m ± 2% 9.455m ± 4% +6.18% (p=0.000 n=10) | ||
| Modsep_N50-8 1.429m ± 3% 1.504m ± 3% +5.29% (p=0.000 n=10) | ||
| geomean 213.2µ 217.1µ +1.84% | ||
|
|
||
| │ baseline.txt │ candidate.txt │ | ||
| │ B/op │ B/op vs base │ | ||
| Reduce-8 52.43Ki ± 0% 52.44Ki ± 0% +0.02% (p=0.002 n=10) | ||
| DiscretizeZOH-8 21.67Ki ± 0% 21.67Ki ± 0% ~ (p=0.696 n=10) | ||
| ControllabilityStaircase-8 22.16Ki ± 0% 22.15Ki ± 0% ~ (p=0.126 n=10) | ||
| Simulate_DCMotor-8 19.67Ki ± 0% 16.28Ki ± 0% -17.22% (p=0.000 n=10) | ||
| Lyap_N100-8 373.0Ki ± 0% 373.3Ki ± 0% +0.08% (p=0.000 n=10) | ||
| Reg_N100_M5_P5-8 424.2Ki ± 0% 421.4Ki ± 0% -0.66% (p=0.000 n=10) | ||
| D2C_ZOH_N20-8 77.60Ki ± 1% 77.40Ki ± 0% -0.25% (p=0.035 n=10) | ||
| D2C_ZOH_N50-8 463.5Ki ± 0% 463.2Ki ± 0% ~ (p=0.218 n=10) | ||
| MatLog_N20-8 64.54Ki ± 0% 64.50Ki ± 0% ~ (p=0.954 n=10) | ||
| MatLog_N50-8 391.2Ki ± 0% 391.8Ki ± 0% +0.16% (p=0.007 n=10) | ||
| SystemFRD_SISO_10000-8 922.6Ki ± 0% 922.6Ki ± 0% +0.01% (p=0.000 n=10) | ||
| Canon_Modal_N50-8 209.7Ki ± 0% 211.0Ki ± 0% +0.58% (p=0.001 n=10) | ||
| Stabsep_N10-8 29.33Ki ± 0% 29.32Ki ± 0% ~ (p=0.288 n=10) | ||
| Stabsep_N50-8 635.1Ki ± 0% 635.4Ki ± 0% ~ (p=0.796 n=10) | ||
| Stabsep_N100-8 2.528Mi ± 0% 2.532Mi ± 0% ~ (p=0.218 n=10) | ||
| Modsep_N50-8 640.9Ki ± 0% 640.3Ki ± 0% ~ (p=0.218 n=10) | ||
| geomean 168.6Ki 166.6Ki -1.18% | ||
|
|
||
| │ baseline.txt │ candidate.txt │ | ||
| │ allocs/op │ allocs/op vs base │ | ||
| Reduce-8 170.0 ± 0% 170.0 ± 0% ~ (p=1.000 n=10) ¹ | ||
| DiscretizeZOH-8 28.00 ± 0% 28.00 ± 0% ~ (p=1.000 n=10) ¹ | ||
| ControllabilityStaircase-8 109.0 ± 0% 109.0 ± 0% ~ (p=1.000 n=10) ¹ | ||
| Simulate_DCMotor-8 45.00 ± 0% 11.00 ± 0% -75.56% (p=0.000 n=10) | ||
| Lyap_N100-8 363.0 ± 0% 363.0 ± 0% ~ (p=0.474 n=10) | ||
| Reg_N100_M5_P5-8 55.00 ± 0% 25.00 ± 0% -54.55% (p=0.000 n=10) | ||
| D2C_ZOH_N20-8 50.00 ± 0% 50.00 ± 0% ~ (p=1.000 n=10) ¹ | ||
| D2C_ZOH_N50-8 53.00 ± 0% 53.00 ± 0% ~ (p=1.000 n=10) ¹ | ||
| MatLog_N20-8 30.00 ± 0% 30.00 ± 0% ~ (p=1.000 n=10) ¹ | ||
| MatLog_N50-8 32.00 ± 0% 32.00 ± 0% ~ (p=1.000 n=10) ¹ | ||
| SystemFRD_SISO_10000-8 60.00 ± 2% 61.00 ± 0% +1.67% (p=0.001 n=10) | ||
| Canon_Modal_N50-8 112.0 ± 0% 112.0 ± 0% ~ (p=1.000 n=10) ¹ | ||
| Stabsep_N10-8 79.00 ± 0% 79.00 ± 0% ~ (p=1.000 n=10) ¹ | ||
| Stabsep_N50-8 156.0 ± 1% 156.0 ± 0% ~ (p=0.211 n=10) | ||
| Stabsep_N100-8 481.0 ± 0% 482.0 ± 0% +0.21% (p=0.002 n=10) | ||
| Modsep_N50-8 143.0 ± 1% 143.0 ± 0% ~ (p=0.087 n=10) | ||
| geomean 84.50 73.74 -12.73% | ||
| ¹ all samples are equal |
Binary file not shown.
Binary file not shown.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When contributors run the inspected macOS
Downstream consumeraction,.codex/environments/environment.toml:89still invokesgo mod edit -replace=gonum.org/v1/gonum=github.com/jamestjsp/gonum@v0.17.3-fork;go help mod editconfirms that the right-hand@vselects that exact replacement version. Consequently, the local action no longer exercises the newly documented v0.19.0 setup and can pass despite a downstream incompatibility with the supported fork.Useful? React with 👍 / 👎.