Skip to content

Fix ZeroDivisionError when an optimization batch extracts nothing #846

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
GeorgWa wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix ZeroDivisionError when an optimization batch extracts nothing #846

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
051b631
Give the optimizer feature a home of its own
Aug 30, 2026
f73ac57
Address code review of the optimizer feature refactor
Aug 30, 2026
2eb4e33
Drop the _feature construction check and simplify the wording
Aug 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 68 additions & 65 deletions alphadia/workflow/optimizers/automatic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from abc import ABC, abstractmethod
from abc import ABC

import matplotlib.pyplot as plt
import numpy as np
Expand All @@ -14,6 +14,10 @@
from alphadia.workflow.managers.fdr_manager import FDRManager
from alphadia.workflow.managers.optimization_manager import OptimizationManager
from alphadia.workflow.optimizers.base import BaseOptimizer
from alphadia.workflow.optimizers.features import (
MeanIsotopeIntensityCorrelation,
PrecursorProportionDetected,
)
from alphadia.workflow.optimizers.optimization_lock import OptimizationLock


Expand Down Expand Up @@ -101,7 +105,11 @@ def step(
verbosity="progress",
)

self._update_history(precursors_df, fragments_df)
if not self._update_history(precursors_df, fragments_df):
# Without a measurement there is no value to converge on. Also, an empty
# frame calibrates the next parameter to zero or NaN. Then all subsequent
# searches find nothing. Thus keep the parameter at its current value.
return

if self._just_converged:
self.has_converged = True
Expand Down Expand Up @@ -144,12 +152,19 @@ def skip(self):

def plot(self):
"""Plot the value of the feature used to assess optimization progress against the parameter value, for each value tested."""
if self.history_df.empty:
self._reporter.log_string(
f"{self.parameter_name}: there is no measurement to plot.",
verbosity="warning",
)
return

fig, ax = plt.subplots()

ax.vlines(
x=self._optimization_manager.__dict__[self.parameter_name],
ymin=0,
ymax=self.history_df.loc[self._find_index_of_optimum(), self._feature_name],
ymax=self.history_df.loc[self._find_index_of_optimum(), self._feature.name],
color="red",
zorder=0,
label=f"Optimal {self.parameter_name}",
Expand All @@ -159,16 +174,16 @@ def plot(self):
sorted_history_df = self.history_df.sort_values("parameter")
ax.plot(
sorted_history_df["parameter"],
sorted_history_df[self._feature_name],
sorted_history_df[self._feature.name],
)
ax.scatter(
self.history_df["parameter"],
self.history_df[self._feature_name],
self.history_df[self._feature.name],
)

ax.set_xlabel(self.parameter_name)
ax.xaxis.set_inverted(True)
ax.set_ylim(bottom=0, top=self.history_df[self._feature_name].max() * 1.1)
ax.set_ylim(bottom=0, top=self.history_df[self._feature.name].max() * 1.1)
ax.legend(loc="upper left")

plt.show()
Expand Down Expand Up @@ -200,27 +215,41 @@ def _propose_new_parameter(self, df: pd.DataFrame):
).ci(df, self.update_percentile_range)
)

def _update_history(self, precursors_df: pd.DataFrame, fragments_df: pd.DataFrame):
"""This method updates the history dataframe with relevant values.

Parameters
----------
precursors_df: pd.DataFrame
The filtered precursor dataframe for the search.
def _update_history(
self, precursors_df: pd.DataFrame, fragments_df: pd.DataFrame
) -> bool:
"""See base class.

fragments_df: pd.DataFrame
The filtered fragment dataframe for the search.
Returns
-------
bool
True if this method recorded a measurement, False if it did not.

"""
feature_value = self._feature.measure(
precursors_df, fragments_df, self._optlock
)

# A round with no measurement is different from a measurement of zero. A
# substitute value of zero makes the relative calculations in _just_converged
# and the axis limits in plot() infinite or NaN. The code does not raise an
# error and the fault stays hidden.
if feature_value is None:
self._reporter.log_string(
f"{self.parameter_name}: {self._feature.name} has no definition for "
f"batch {self._optlock.batch_idx}. No measurement was recorded for "
f"this round.",
verbosity="warning",
)
return False

new_row = pd.DataFrame(
[
{
"parameter": self._optimization_manager.__dict__[
self.parameter_name
],
self._feature_name: self._get_feature_value(
precursors_df, fragments_df
),
self._feature.name: feature_value,
"classifier_version": self._fdr_manager.current_version, # TODO: only we need from fdr_manager
"score_cutoff": self._optimization_manager.score_cutoff,
"fwhm_rt": self._optimization_manager.fwhm_rt,
Expand All @@ -230,6 +259,7 @@ def _update_history(self, precursors_df: pd.DataFrame, fragments_df: pd.DataFram
]
)
self.history_df = pd.concat([self.history_df, new_row], ignore_index=True)
return True

@property
def _batch_substantially_bigger(self):
Expand Down Expand Up @@ -269,7 +299,7 @@ def _just_converged(self):
if len(self.history_df) < 3:
return False

feature_history = self.history_df[self._feature_name]
feature_history = self.history_df[self._feature.name]
last_feature_value = feature_history.iloc[-1]
second_last_feature_value = feature_history.iloc[-2]
third_last_feature_value = feature_history.iloc[-3]
Expand Down Expand Up @@ -339,24 +369,24 @@ def _find_index_of_optimum(self) -> int:
return self.history_df.index[0]

if self._favour_narrower_optimum: # This setting can be useful for optimizing parameters for which many parameter values have similar feature values.
maximum_feature_value = self.history_df[self._feature_name].max()
maximum_feature_value = self.history_df[self._feature.name].max()
threshold = (
maximum_feature_value
- self._maximum_decrease_from_maximum * np.abs(maximum_feature_value)
)

rows_within_thresh_of_max = self.history_df[
self.history_df[self._feature_name] > threshold
self.history_df[self._feature.name] > threshold
]

if rows_within_thresh_of_max.empty:
# If no rows meet the threshold, return the index of the max feature value
return self.history_df[self._feature_name].idxmax()
return self.history_df[self._feature.name].idxmax()
else:
return rows_within_thresh_of_max["parameter"].idxmin()

else:
return self.history_df[self._feature_name].idxmax()
return self.history_df[self._feature.name].idxmax()

def _update_workflow(self):
"""Updates the optimization manager with the results of the optimization, namely:
Expand All @@ -368,6 +398,13 @@ def _update_workflow(self):
at the optimal parameter. Also updates the optlock with the batch index at the optimum.

"""
if self.history_df.empty:
self._reporter.log_string(
f"{self.parameter_name}: no round gave a measurement. The current value stays.",
verbosity="warning",
)
return

index_of_optimum = self._find_index_of_optimum()

optimal_parameter = self.history_df["parameter"].loc[index_of_optimum]
Expand Down Expand Up @@ -396,26 +433,10 @@ def _update_workflow(self):
# The time impact of this is negligible and the benefits can be significant.
self._optlock.batch_idx = batch_index_at_optimum

@abstractmethod
def _get_feature_value(
self, precursors_df: pd.DataFrame, fragments_df: pd.DataFrame
):
"""Each parameter is optimized according to a particular feature. This method gets the value of that feature for a given round of optimization.

Parameters
----------

precursors_df: pd.DataFrame
The precursor dataframe for the search

fragments_df: pd.DataFrame
The fragment dataframe for the search


"""


class AutomaticRTOptimizer(AutomaticOptimizer):
_feature = PrecursorProportionDetected()

def __init__(
self,
initial_parameter: float,
Expand All @@ -430,7 +451,6 @@ def __init__(
self.parameter_name = "rt_error"
self._estimator_group_name = CalibrationGroups.PRECURSOR
self._estimator_name = CalibrationEstimators.RT
self._feature_name = "precursor_proportion_detected"
super().__init__(
initial_parameter,
config,
Expand All @@ -441,13 +461,10 @@ def __init__(
reporter,
)

def _get_feature_value(
self, precursors_df: pd.DataFrame, fragments_df: pd.DataFrame
):
return len(precursors_df) / self._optlock.total_elution_groups


class AutomaticMS2Optimizer(AutomaticOptimizer):
_feature = PrecursorProportionDetected()

def __init__(
self,
initial_parameter: float,
Expand All @@ -462,7 +479,6 @@ def __init__(
self.parameter_name = "ms2_error"
self._estimator_group_name = CalibrationGroups.FRAGMENT
self._estimator_name = CalibrationEstimators.MZ
self._feature_name = "precursor_proportion_detected"
super().__init__(
initial_parameter,
config,
Expand All @@ -473,13 +489,10 @@ def __init__(
reporter,
)

def _get_feature_value(
self, precursors_df: pd.DataFrame, fragments_df: pd.DataFrame
):
return len(precursors_df) / self._optlock.total_elution_groups


class AutomaticMS1Optimizer(AutomaticOptimizer):
_feature = MeanIsotopeIntensityCorrelation()

def __init__(
self,
initial_parameter: float,
Expand All @@ -494,7 +507,6 @@ def __init__(
self.parameter_name = "ms1_error"
self._estimator_group_name = CalibrationGroups.PRECURSOR
self._estimator_name = CalibrationEstimators.MZ
self._feature_name = "mean_isotope_intensity_correlation"
super().__init__(
initial_parameter,
config,
Expand All @@ -505,13 +517,10 @@ def __init__(
reporter,
)

def _get_feature_value(
self, precursors_df: pd.DataFrame, fragments_df: pd.DataFrame
):
return precursors_df["isotope_intensity_correlation"].mean()


class AutomaticMobilityOptimizer(AutomaticOptimizer):
_feature = PrecursorProportionDetected()

def __init__(
self,
initial_parameter: float,
Expand All @@ -526,7 +535,6 @@ def __init__(
self.parameter_name = "mobility_error"
self._estimator_group_name = CalibrationGroups.PRECURSOR
self._estimator_name = CalibrationEstimators.MOBILITY
self._feature_name = "precursor_proportion_detected"
super().__init__(
initial_parameter,
config,
Expand All @@ -536,8 +544,3 @@ def __init__(
optlock,
reporter,
)

def _get_feature_value(
self, precursors_df: pd.DataFrame, fragments_df: pd.DataFrame
):
return len(precursors_df) / self._optlock.total_elution_groups
12 changes: 10 additions & 2 deletions alphadia/workflow/optimizers/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@
from alphadia.workflow.managers.calibration_manager import CalibrationManager
from alphadia.workflow.managers.fdr_manager import FDRManager
from alphadia.workflow.managers.optimization_manager import OptimizationManager
from alphadia.workflow.optimizers.features import OptimizationFeature


class BaseOptimizer(ABC):
parameter_name: str | None
_estimator_name: str | None
_estimator_group_name: str | None
_feature_name: str | None
_feature: OptimizationFeature

def __init__(
self,
Expand Down Expand Up @@ -92,7 +93,9 @@ def _update_workflow(self):
"""

@abstractmethod
def _update_history(self, precursors_df: pd.DataFrame, fragments_df: pd.DataFrame):
def _update_history(
self, precursors_df: pd.DataFrame, fragments_df: pd.DataFrame
) -> bool:
"""This method updates the history dataframe with relevant values.

Parameters
Expand All @@ -103,4 +106,9 @@ def _update_history(self, precursors_df: pd.DataFrame, fragments_df: pd.DataFram
fragments_df: pd.DataFrame
The filtered fragment dataframe for the search.

Returns
-------
bool
True if this method recorded a measurement, False if it did not.

"""
Loading
Loading