Skip to content

Comments

feat: Add max_iter stopping criteria and refactor sampling API#18

Draft
pFornagiel wants to merge 5 commits intomainfrom
@pFornagiel/add-n-iterations
Draft

feat: Add max_iter stopping criteria and refactor sampling API#18
pFornagiel wants to merge 5 commits intomainfrom
@pFornagiel/add-n-iterations

Conversation

@pFornagiel
Copy link
Collaborator

Description

This PR introduces a max_iter stopping criterion for Basin-Hopping sampling, which caps the total number of iterations per run, and renames max_perturbations_without_improvement to n_iters_no_change to align API naming with the one of scikit libraries.

It also simplifies the compute_lon public API by merging configuration parameters into a single BasinHoppingSamplerConfig object, consistent with how BasinHoppingSampler is already used directly.

Fixes #13

Changes made

src/lonpy/sampling.py

BasinHoppingSamplerConfig

  • Renamed max_perturbations_without_improvement to n_iter_no_change (int | None, default 1000):

    • The new name aligns with the scikit-learn convention and describes the parameter more clearly (consecutive non-improving iterations, not raw perturbation count).
    • Made nullable (None) to allow disabling this criterion entirely when max_iter is set.
  • Added max_iter: int | None = None:

    • A new optional cap on the total number of perturbation steps per run, regardless of improvement.
  • Added __post_init__ validation:

    • Raises ValueError if both n_iter_no_change and max_iter are None, ensuring at least one stopping criterion is always active.
  • Improved docstrings for minimizer_method and minimizer_options to include cross-references to scipy.optimize.minimize documentation and clarify accepted argument types.

BasinHoppingSampler._basin_hopping_sampling()

  • Replaced the fixed while perturbations_without_improvement < ... loop condition with while True: and explicit break statements for both stopping criteria.

compute_lon()

  • Replaced the following flat parameters:

    • seed, step_size, step_mode, n_runs, max_perturbations_without_improvement, fitness_precision, coordinate_precision, bounded

    with a single config: BasinHoppingSamplerConfig | None = None parameter.

    This makes compute_lon consistent with BasinHoppingSampler.sample_to_lon() and removes the risk of the two APIs diverging during the development. We did not accept all the possible options already and constructed the config itself inside the method, so this seems like a right decision in my opinion.

  • Updated docstring accordingly.

Docs

Documentation has been updated to reflect all of the above API changes.

@pFornagiel pFornagiel changed the title Add max_iter stopping criteria and refactor sampling API feat: Add max_iter stopping criteria and refactor sampling API Feb 23, 2026
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.

Revert or add additional n_iterations-based termination in BasinHoppingSamplerConfig

1 participant