ICB-DCM / pyPESTO

python Parameter EStimation TOolbox
https://pypesto.readthedocs.io
BSD 3-Clause "New" or "Revised" License
207 stars 45 forks source link

Handling of simulation failures in AmiciPredictor #1359

Open dweindl opened 3 months ago

dweindl commented 3 months ago

Currently, when AmiciPredictor processes a failed simulation, it sets the timepoints to NaN: https://github.com/ICB-DCM/pyPESTO/blob/82b7d5da1273b9d442a634e5aabbda9685279d77/pypesto/predict/amici_predictor.py#L375-L377

This is rather inconvenient, since those NaNs cause issues in pypesto.visualize.sampling.sampling_prediction_trajectories later on:

File /venv/lib/python3.11/site-packages/pypesto/visualize/sampling.py:901, in sampling_prediction_trajectories(ensemble_prediction, levels, title, size, axes, labels, axis_label_padding, groupby, condition_gap, condition_ids, output_ids, weighting, reverse_opacities, average, add_sd, measurement_df)
    887     _plot_trajectories_by_condition(
    888         summary=summary,
    889         condition_ids=condition_ids,
   (...)
    898         grouped_measurements=grouped_measurements,
    899     )
    900 elif groupby == OUTPUT:
--> 901     _plot_trajectories_by_output(
    902         summary=summary,
    903         condition_ids=condition_ids,
    904         output_ids=output_ids,
    905         axes=axes,
    906         levels=levels,
    907         level_opacities=level_opacities,
    908         labels=labels,
    909         variable_colors=variable_colors,
    910         average=average,
    911         add_sd=add_sd,
    912         grouped_measurements=grouped_measurements,
    913     )
    915 if title:
    916     fig.suptitle(title)

File /venv/lib/python3.11/site-packages/pypesto/visualize/sampling.py:435, in _plot_trajectories_by_output(summary, condition_ids, output_ids, axes, levels, level_opacities, labels, variable_colors, average, add_sd, grouped_measurements)
    432 # Timepoints must match, or `upper_data` will be plotted at
    433 # some incorrect time points.
    434 if not (np.array(t_lower) == np.array(t_upper)).all():
--> 435     raise ValueError(
    436         "The timepoints of the data for the upper and lower "
    437         "percentiles do not match."
    438     )
    439 # Plot a shaded region between the data that correspond to the
    440 # lower and upper percentiles.
    441 ax.fill_between(
    442     t_lower_shifted,
    443     lower_data,
   (...)
    448     lw=0,
    449 )

ValueError: The timepoints of the data for the upper and lower percentiles do not match.  

I see no harm in using the actual timepoints instead of NaNs in the case of failed simulations and would suggest doing that.