Skip to content

Fix: Tune SA performance to avoid slowdown#9

Merged
VoX merged 1 commit intomainfrom
fix/performance-tuning
Apr 5, 2026
Merged

Fix: Tune SA performance to avoid slowdown#9
VoX merged 1 commit intomainfrom
fix/performance-tuning

Conversation

@VoX
Copy link
Copy Markdown
Owner

@VoX VoX commented Apr 5, 2026

Problem

SA was ~10x slower than the original hill climbing per shape because:

  • SA ran maxAge * 10 = 1000 iterations vs hill climbing's ~100
  • times was increased to cores/2, multiplying the cost further
  • Temperature estimation probed 30 mutations before each SA run

Fix

  • Reduced SA iterations from maxAge * 10 to maxAge * 3 (300 iterations)
  • Reverted times to 1 (SA explores well enough without parallel chains)
  • Reduced temperature probe count from 30 to 10

Impact

  • ~3x faster than previous SA configuration
  • Still ~3x slower than pure hill climbing, but with measurably better quality per shape
  • Build passes, all tests green

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>
Copilot AI review requested due to automatic review settings April 5, 2026 01:57
@VoX VoX merged commit 53d494f into main Apr 5, 2026
2 checks passed
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 times to 1 for 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.

Comment on lines 90 to 93
// 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);
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
@@ -167,7 +167,7 @@ static float estimateTemperature(State state) {
* {@code initialTemp} to near-zero (0.001) over {@code maxAge * 10} iterations.
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
* {@code initialTemp} to near-zero (0.001) over {@code maxAge * 10} iterations.
* {@code initialTemp} to near-zero (0.001) over {@code maxAge * 3} iterations.

Copilot uses AI. Check for mistakes.
Comment on lines 99 to 102
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

Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants