Skip to content

Refactor GeneticAlgorithm to accept generations as a parameter in run_evolution#45

Open
JacobBorden wants to merge 2 commits into
masterfrom
refactor-ga-generations-param-6766763341250922330
Open

Refactor GeneticAlgorithm to accept generations as a parameter in run_evolution#45
JacobBorden wants to merge 2 commits into
masterfrom
refactor-ga-generations-param-6766763341250922330

Conversation

@JacobBorden

Copy link
Copy Markdown
Owner

This PR addresses an issue where the num_generations was passed to the GeneticAlgorithm constructor and held as an internal state variable. The refactoring instead makes num_generations an explicit parameter when calling the run_evolution method. This allows an existing GeneticAlgorithm instance to cleanly run varying numbers of generations on separate evolution calls without re-instantiating.

  • The constructor signature is changed in src/optimization/genetic_algorithm.h and .cpp.
  • The run_evolution method signature is changed.
  • Documentation and code examples have been updated.
  • Benchmarks and unit tests have been updated and are all successfully passing.

PR created automatically by Jules for task 6766763341250922330 started by @JacobBorden

- Removes `num_generations` from the `Optimization::GeneticAlgorithm` constructor.
- Adds `num_generations` as the first parameter to the `run_evolution` method.
- Updates all callers including `tests/test_benchmarks.cpp`, `tests/test_genetic_algorithm.cpp`, and `examples/basic_usage_export.cpp`.
- Specifically resolves the hardcoded generations workaround in `tests/test_genetic_algorithm.cpp`.
- Updates `README.md` and `docs/modules/optimization.md` to reflect these signature changes.

Co-authored-by: JacobBorden <22827637+JacobBorden@users.noreply.github.com>
@google-labs-jules

Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Copy link
Copy Markdown
Owner Author

PR steward note: leaving this blocked for a maintainer decision. The change is a public API shift for GeneticAlgorithm (num_generations moves from constructor state to a run_evolution argument), the branch is conflicting on current master, and the historical CMake check is red. This is more than a mechanical stale-branch repair because it changes the documented call shape and downstream examples/tests.

@google-labs-jules

Copy link
Copy Markdown
Contributor

PR steward note: leaving this blocked for a maintainer decision. The change is a public API shift for GeneticAlgorithm (num_generations moves from constructor state to a run_evolution argument), the branch is conflicting on current master, and the historical CMake check is red. This is more than a mechanical stale-branch repair because it changes the documented call shape and downstream examples/tests.

Understood. The changes address the requested refactor to make num_generations a parameter of run_evolution, which does represent a public API shift. The code, tests, and documentation have been updated accordingly, and the tests pass locally. Awaiting maintainer decision.

- Removes `num_generations` from the `Optimization::GeneticAlgorithm` constructor.
- Adds `num_generations` as the first parameter to the `run_evolution` method.
- Updates all callers including `tests/test_benchmarks.cpp`, `tests/test_genetic_algorithm.cpp`, and `examples/basic_usage_export.cpp`.
- Specifically resolves the hardcoded generations workaround in `tests/test_genetic_algorithm.cpp`.
- Updates `README.md` and `docs/modules/optimization.md` to reflect these signature changes.

Co-authored-by: JacobBorden <22827637+JacobBorden@users.noreply.github.com>

@JacobBorden JacobBorden left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

PR steward recheck for 2026-06-12: this remains blocked on a maintainer API decision. The diff moves num_generations from GeneticAlgorithm constructor state into run_evolution(...), updates docs/examples/tests around that public call shape, and the branch is still conflicting with current master. I am leaving it open rather than resolving conflicts in a direction that could break downstream callers without maintainer approval.

@JacobBorden JacobBorden left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

PR steward recheck for 2026-06-15: this remains blocked for the same maintainer API decision. The branch changes the public GeneticAlgorithm call shape by moving num_generations into run_evolution(...), is still stale/conflicting against current master, and has no fresh visible checks. I am leaving it open rather than resolving conflicts without an explicit direction on the API change.

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.

1 participant