chore: update Gonum fork and require Go 1.27.1 - #171
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54d5d6b119
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| require gonum.org/v1/gonum v0.15.0 | ||
|
|
||
| replace gonum.org/v1/gonum => github.com/jamestjsp/gonum v0.17.7-fork | ||
| replace gonum.org/v1/gonum => github.com/jamestjsp/gonum v0.19.0-fork |
There was a problem hiding this comment.
This changes the numerical backend and brings in intervening BLAS changes, but no before/after benchmark data accompanies the upgrade, so performance regressions in matrix-heavy operations could ship unnoticed. Run go test -bench=. -benchmem against the parent and upgraded versions and compare the results with benchstat before landing.
AGENTS.md reference: AGENTS.md:L35-L37
Useful? React with 👍 / 👎.
| go mod edit -require=github.com/jamestjsp/controlsys@v0.0.0 | ||
| go mod edit -replace=github.com/jamestjsp/controlsys="$GITHUB_WORKSPACE" | ||
| go mod edit -replace=gonum.org/v1/gonum=github.com/jamestjsp/gonum@v0.17.3-fork | ||
| go mod edit -replace=gonum.org/v1/gonum=github.com/jamestjsp/gonum@v0.19.0-fork |
There was a problem hiding this comment.
When contributors run the inspected macOS Downstream consumer action, .codex/environments/environment.toml:89 still invokes go mod edit -replace=gonum.org/v1/gonum=github.com/jamestjsp/gonum@v0.17.3-fork; go help mod edit confirms that the right-hand @v selects 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 👍 / 👎.
jamestjsp
changed the title
1 participant
Updates the required Gonum fork from v0.17.7-fork to v0.19.0-fork and raises the minimum Go version from 1.26.3 to 1.27.1. README installation guidance, CI, and the local downstream-consumer action use the same Gonum pin. The Go 1.27 go-fix rewrite uses slices.Backward for ModelArray indexing.
Go 1.27.1 substantially reduces the large regressions observed with Go 1.26.4. A full 236-case comparison, ten interleaved samples per version, improves timing geomean by 1.07%. Longer follow-up confirms remaining slowdowns: SISO FRD +6.17%, internal-delay simulation +4.38%, SISO delayed simulation +3.38%. Original MatLog/D2C/Stabsep regressions are now approximately 0.5–1.1%; simulation and regulator gains remain.
Complete Go 1.27.1 report and retained evidence. The comparison holds all 191 Go source files identical, including the new go directive and go-fix rewrite.
Investigation of Go 1.26.4 layout sensitivity. Gonum issue #10 remains open; no production Gonum numerical changes are included.
Validation on Go 1.27.1: full uncached tests, race tests, go fix, go vet, go build, local external-consumer action, and GitHub CI passed. CLI govulncheck v1.8.0 reports no vulnerabilities; the bundled gopls scanner is too old to process Go 1.27.
Release readiness: v1.5.1 remains an unpublished draft. Assess or repair the residual 3–6% performance regressions before merge/publication. The Go 1.27.1 minimum requirement is an intentional compatibility change. Performance evidence is limited to Apple M1 Pro / darwin-arm64 / eight workers.