Add tuning search based on CompileIQ#9190
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThree changes add multithreaded benchmark execution support: ProcessRunner now guards signal handler registration to the main thread, SQLiteStorage validates thread-safety and enables cross-thread connection use, and a new CompileIQSeeker orchestrator selects between brute-force and evolutionary search strategies based on problem size. ChangesMultithreaded Benchmark Infrastructure and Search
Suggested reviewers
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
benchmarks/scripts/search_iq.py (1)
120-131: ⚡ Quick winsuggestion: Rename parameter or variable to clarify intent.
Line 125 passes
num_rt_workloadsto a parameter namednum_objectivesinget_num_expected_runs(). The function signature andiq_search()(line 83) usenum_objectives=1, but the calculation here usesnum_rt_workloads. Either rename the parameter inget_num_expected_runs()to reflect its actual usage, or clarify the relationship between RT workloads and the expected-runs heuristic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 75cb46dd-cbc5-44dd-9b6d-7dd9bc33fb9b
📒 Files selected for processing (3)
benchmarks/scripts/cccl/bench/bench.pybenchmarks/scripts/cccl/bench/storage.pybenchmarks/scripts/search_iq.py
| ) | ||
|
|
||
| if score == float("inf") or score == float("-inf"): | ||
| print("Infinite store") |
There was a problem hiding this comment.
important: Fix typo "Infinite store" → "Infinite score".
Line 60 prints "Infinite store" but should print "Infinite score" to match the condition being checked.
|
ok, here is a confusing bit. Yet, the scores reported to CompileIQ are (ordered by variant as the list above):
@gevtushenko as the author of the tuning framework, I kindly ask for an explanation for this observation. |
| def pool_cull_sizes(num_genes, num_objectives, variant_space_size, cull=0.75): | ||
| min_pool_size = 128 if variant_space_size > 10000 else 32 | ||
| target = (2 * num_objectives) + 1 | ||
| poolsize = int(target / (1 - cull)) |
There was a problem hiding this comment.
I would clamp cull value to between 0.05 and 0.95 (or some boundaries within the unit interval) or validate this argument and raise ValueError is not found within this range.
| target = (2 * num_objectives) + 1 | ||
| poolsize = int(target / (1 - cull)) | ||
| poolsize = max(max(poolsize, min_pool_size), 2 * num_genes) | ||
| poolsize = poolsize if poolsize % 2 == 0 else poolsize + 1 |
There was a problem hiding this comment.
Nit:
| poolsize = poolsize if poolsize % 2 == 0 else poolsize + 1 | |
| poolsize = poolsize + (poolsize % 2) |
Or use ((poolsize + 1) // 2) * 2.
| poolsize = max(max(poolsize, min_pool_size), 2 * num_genes) | ||
| poolsize = poolsize if poolsize % 2 == 0 else poolsize + 1 | ||
| cullsize = int(poolsize * cull) | ||
| cullsize = cullsize if cullsize % 2 == 0 else cullsize - 1 |
There was a problem hiding this comment.
Nit:
| cullsize = cullsize if cullsize % 2 == 0 else cullsize - 1 | |
| cullsize = cullsize - (cullsize % 2) |
or cullsize = (cullsize // 2) * 2.
| for rng in parameter_space: | ||
| search_space[rng.label] = ss.range( | ||
| start=rng.low, end=rng.high - 1, step=rng.step | ||
| ) |
There was a problem hiding this comment.
Variable rng evokes association with "random number generator", while range is likely intended. Since range is the built-in keyword. Can we use search_range instead?
| for rng in parameter_space: | |
| search_space[rng.label] = ss.range( | |
| start=rng.low, end=rng.high - 1, step=rng.step | |
| ) | |
| for search_range in parameter_space: | |
| search_space[search_range.label] = ss.range( | |
| start=search_range.low, end=search_range.high - 1, step=search_range.step | |
| ) |
| for rng in parameter_space: | ||
| value = int(config[rng.label]) | ||
| range_points.append(bench.RangePoint(rng.definition, rng.label, value)) |
There was a problem hiding this comment.
Ditto here. I would prefer search_range over rng, please.
2 participants
This is mostly done by
claude, trying to migrate the internalcub_tuning_evoscripts. This PR adds a simplified version using a single worker, running benchmarks on a single GPU.Running:
It's still a bit confusing, because after running, the database shows different results, but it looks like the score reported by
analyze.pyis just computed differently than the score passed to compile-iq.