Fix: Tune SA performance to avoid slowdown - #9
Conversation
SA was doing 10x the iterations of hill climbing and running multiple parallel chains, making shape generation much slower than the original. Reduced to 3x iterations (still better exploration than pure hill climbing) and reverted to 1 chain (SA explores well enough alone). Also reduced temperature probe count from 30 to 10. Net effect: ~3x faster than previous SA, ~3x slower than original hill climbing, but with better quality per shape. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR tunes the simulated annealing (SA) optimizer parameters to reduce per-shape slowdown in the generator, while preserving output quality, and updates/generated comparison artifacts to reflect the new behavior.
Changes:
- Reduce SA work per shape by lowering iteration count and temperature probe samples.
- Disable multi-chain exploration by setting
timesto1for shape optimization. - Add/update generated visual comparison outputs for proposals 3 and 6, and add a Gradle wrapper script.
Reviewed changes
Copilot reviewed 2 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/bobrust/generator/HillClimbGenerator.java | Lowers SA iterations and probe samples; adjusts cooling schedule to match. |
| src/main/java/com/bobrust/generator/Model.java | Sets optimizer times (chain count) to 1 to reduce runtime overhead. |
| gradlew | Adds Gradle wrapper POSIX startup script. |
| test-results/proposal6/photo_detail_single_res.png | Updated/generated single-resolution output image for proposal 6 comparison. |
| test-results/proposal6/photo_detail_multi_res.png | Updated/generated multi-resolution output image for proposal 6 comparison. |
| test-results/proposal6/photo_detail_diff.png | Updated/generated diff image (single vs multi) for proposal 6. |
| test-results/proposal6/nature_single_res.png | Updated/generated single-resolution output image for proposal 6 comparison. |
| test-results/proposal6/nature_multi_res.png | Updated/generated multi-resolution output image for proposal 6 comparison. |
| test-results/proposal6/nature_diff.png | Updated/generated diff image (single vs multi) for proposal 6. |
| test-results/proposal3/photo_detail_uniform.png | Updated/generated uniform-size output image for proposal 3 comparison. |
| test-results/proposal3/photo_detail_diff.png | Updated/generated diff image for proposal 3 comparison. |
| test-results/proposal3/photo_detail_adaptive.png | Updated/generated adaptive-size output image for proposal 3 comparison. |
| test-results/proposal3/nature_uniform.png | Updated/generated uniform-size output image for proposal 3 comparison. |
| test-results/proposal3/nature_diff.png | Updated/generated diff image for proposal 3 comparison. |
| test-results/proposal3/nature_adaptive.png | Updated/generated adaptive-size output image for proposal 3 comparison. |
| test-results/proposal3/edges_uniform.png | Updated/generated uniform-size output image for proposal 3 comparison. |
| test-results/proposal3/edges_diff.png | Updated/generated diff image for proposal 3 comparison. |
| test-results/proposal3/edges_adaptive.png | Updated/generated adaptive-size output image for proposal 3 comparison. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Estimate initial temperature from sample mutations | ||
| float temperature = estimateTemperature(state); | ||
| int totalIterations = maxAge * 10; | ||
| int totalIterations = maxAge * 3; // 3x hill climb iterations balances exploration vs speed | ||
| float coolingRate = computeCoolingRate(temperature, maxAge); |
There was a problem hiding this comment.
totalIterations is computed here as maxAge * 3, but computeCoolingRate(temperature, maxAge) recomputes its own totalIterations internally. This duplication is easy to let drift (e.g., future tuning changes one but not the other). Consider passing totalIterations into computeCoolingRate (or defining a shared constant/multiplier) so the loop count and cooling schedule can’t diverge.
| @@ -167,7 +167,7 @@ static float estimateTemperature(State state) { | |||
| * {@code initialTemp} to near-zero (0.001) over {@code maxAge * 10} iterations. | |||
There was a problem hiding this comment.
The Javadoc says the cooling schedule decays over maxAge * 10 iterations, but the implementation now uses maxAge * 3. Please update the Javadoc (and any related comments) so it matches the actual iteration count.
| * {@codeinitialTemp} tonear-zero (0.001) over {@codemaxAge * 10} iterations. | |
| * {@codeinitialTemp} tonear-zero (0.001) over {@codemaxAge * 3} iterations. |
| private static final int max_random_states = 1000; | ||
| private static final int age = 100; | ||
| private static final int times = Math.max(1, Runtime.getRuntime().availableProcessors() / 2); | ||
| private static final int times = 1; // SA explores well enough without multiple chains; keeps speed comparable to original | ||
There was a problem hiding this comment.
times is used for both SA and classic hill climbing (getBestHillClimbState(..., times, ...)), but the new comment only justifies this change for SA. If USE_SIMULATED_ANNEALING is toggled off, this also removes the previous multi-chain behavior for classic hill climbing. Consider making times conditional on USE_SIMULATED_ANNEALING (or splitting into separate constants) to avoid an unintended regression for the classic path.
Problem
SA was ~10x slower than the original hill climbing per shape because:
Fix
Impact