gpl: register large placement tests in bazel and ctest#10962
Conversation
The ./regression-large suite (macro01-03, medium01-06, large01-02) was only runnable via the standalone tclsh script. Register the tests in both build systems: - Bazel: manual-tagged regression_test targets (size = "large") so wildcard builds and CI skip them; run explicitly, e.g. bazelisk test //src/gpl/test:large01-tcl_test - CMake: guarded by -DGPL_LARGE_TESTS=ON (off by default) so bare ctest and coverage runs are unaffected. Refresh the stale .ok/.defok goldens: all 11 tests run to completion and the final HPWL moves by at most 0.04% versus the old goldens. Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request integrates a suite of long-running placement tests (LARGE_TESTS) into both the Bazel and CMake build configurations, tagging them as manual or optional to prevent them from running during standard CI cycles. It also updates the golden output files (.ok) to reflect minor numerical changes in global placement iterations and final HPWL values. The feedback recommends replacing a list comprehension in the Bazel BUILD file with a standard for loop, as using list comprehensions solely for side-effects is discouraged in Starlark.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Matt Liberty <matt.liberty@gmail.com>
|
@codex review |
![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: 7585f03f21
ℹ️ 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".
| data = [":test_resources"], | ||
| ) for test_name in ALL_TESTS] | ||
|
|
||
| for test_name in LARGE_TESTS: |
There was a problem hiding this comment.
In this context, src/gpl/test/BUILD can no longer be loaded by Bazel because BUILD files disallow top-level for statements; Bazel’s current Starlark docs say to use functions instead and note that list comprehensions are allowed in BUILD files (https://bazel.build/rules/language). This means any bazel test //src/gpl/test:..., including existing small GPL tests, fails during package loading before the new manual targets can run. Please use the same list-comprehension pattern as the existing ALL_TESTS registration or move the loop into a .bzl macro.
Useful? React with 👍 / 👎.
|
This is good, I wasn't able to use threads with the regression-large script, now we can. |
|
I see I can try to fix if you want me to @maliberty |
2 participants
The
./regression-largesuite (macro01-03, medium01-06, large01-02) was only runnable via the standalone tclsh script, and its goldens had gone stale — the suite fails on master. This registers the tests in both build systems and refreshes the goldens:regression_testtargets (size = "large") so wildcard builds and CI skip them; run explicitly, e.g.bazelisk test //src/gpl/test:large01-tcl_test-DGPL_LARGE_TESTS=ON(off by default) so barectestand coverage runs are unaffected. When enabled the tests join thegpllabel and get the sameENABLE_GPU=0pinning as the other golden-compare tests..ok/.defokpairs regenerated (the bulk of the diff). Every test runs to completion; final HPWL moves by at most 0.04% versus the old goldens. Runtimes are 9-150 s per test.medium07andlarge03have.tclfiles but no goldens and are not inregression_tests_large.tcl, so they remain unregistered.