Skip to content

Fix ZeroDivisionError when an optimization batch extracts nothing - #846

Open
GeorgWa wants to merge 3 commits into
mainfrom
fix/optimizer-feature-refactor
Open

Fix ZeroDivisionError when an optimization batch extracts nothing#846
GeorgWa wants to merge 3 commits into
mainfrom
fix/optimizer-feature-refactor

Conversation

@GeorgWa

@GeorgWa GeorgWa commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

A search stops with a ZeroDivisionError when an optimization batch extracts nothing. The failure occurs in the code that must recover from a batch with too few precursors.

Three optimizers measure the same quantity. Each one calculated it separately, and none of them handled a batch with no data. This change gives the measurement its own class. The class holds the name, the calculation, and the rule for a batch with no data. All optimizers now use one rule, and a new optimizer gets the rule with it.

A round with no data is left out of the optimization history. The alternative is to record a zero. But zero is also a valid measurement, and it would corrupt the convergence test without an error message.

Optimization keeps the current search parameter when a round gives no data. It also completes normally when no round gives data at all.

A batch with data behaves as before.

🤖 Generated with Claude Code

GeorgWa and others added 3 commits August 30, 2026 17:48
An optimization batch that extracts nothing sets total_elution_groups to 0,
and the three optimizers computing precursor_proportion_detected each divided
by it unguarded. The crash landed inside
proceed_with_insufficient_precursors, the branch that exists to absorb exactly
this case.

Move the feature into workflow/optimizers/features.py, where a class owns its
history column name, its computation, and its definition of undefined. The
guard lives inside the feature, so it cannot be forgotten by a fifth
optimizer, and name and computation can no longer drift apart.

A round without a measurement is skipped rather than recorded as 0.0: a
sentinel would make the relative comparisons in _just_converged and the axis
limits in plot() non-finite, trading a loud failure for a silent one.
_update_workflow keeps the current parameter when no round produced a
measurement, since _find_index_of_optimum raises on an empty history.

Behaviour is unchanged for any batch with total_elution_groups > 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Skipping the history row was not enough: step() carried on to propose a new
parameter from the same empty frame. An unfitted estimator's ci() returns 0, so
the parameter was silently set to 0.0 and every later search extracted nothing
— the loud failure this branch removed, traded for a silent one. _update_history
now reports whether it recorded a measurement, and step() leaves the parameter
alone when it did not.

Also:
- guard plot() against the empty history that _update_workflow already handles,
  so re-enabling its commented-out call site cannot reintroduce the crash
- raise at construction when a subclass declares no _feature, restoring the
  fail-fast that the abstract _get_feature_value used to provide
- treat an all-NaN isotope_intensity_correlation column as unmeasurable, the
  NaN hazard MeanIsotopeIntensityCorrelation's own docstring describes
- word the skip warning for both features, not just the zero-elution-group case

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Remove the hasattr check in AutomaticOptimizer.__init__. A subclass that does
not declare _feature now fails in the same way as one that does not declare
parameter_name or _estimator_name.

Write the new docstrings and comments in simplified technical English
(ASD-STE100): short sentences, active voice, and simple words.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@GeorgWa
GeorgWa requested a review from mschwoer August 31, 2026 07:54
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