Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additional convergence criterion #330

Merged
merged 9 commits into from
Aug 30, 2024

Conversation

odespard
Copy link
Collaborator

@odespard odespard commented Aug 29, 2024

At present, optimizers need to have reached the target number of precursors before they can converge. This poses a problem if the optimizer tries a value so narrow that it is difficult to identify the target number. However, such values are not likely to optimize the feature value (indeed, if the feature value is well chosen they shouldn't, as we should not pick parameter values that reduce precursor counts so much). This PR introduces a new set of methods which record when optimization has been skipped and considers optimization to have converged if the optimizer is skipped too many times (as defined by max_skips in the calibration field of the config). The max_skips field has a default value of 1 and is unlikely to be worth changing as long as we use an exponential batch plan, since skipping an optimizer more than once implies that performance in precursors identifications is at least roughly twice as bad as before.

@odespard odespard requested a review from mschwoer August 29, 2024 09:59
Base automatically changed from backtrack_batch_id to development August 29, 2024 10:00
alphadia/workflow/optimization.py Show resolved Hide resolved
alphadia/workflow/optimization.py Outdated Show resolved Hide resolved
alphadia/workflow/optimization.py Outdated Show resolved Hide resolved
@odespard odespard added the test:e2e End to end tests will be run on PRs that carry this label. label Aug 29, 2024
@odespard odespard merged commit 6e9b5c2 into development Aug 30, 2024
6 of 10 checks passed
@odespard odespard deleted the additional_convergence_criterion branch August 30, 2024 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test:e2e End to end tests will be run on PRs that carry this label.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants