cta-observatory / pyirf

Python IRF builder
https://pyirf.readthedocs.io/en/stable/
MIT License
15 stars 25 forks source link

Documentation for `calculate_percentile_cut` unclear for case of underflows #263

Closed Tobychev closed 11 months ago

Tobychev commented 11 months ago

The API for calculate_percentile_cut contains both a min_value argument and a fill_value argument, both of which claim to be the value which will be assigned to events with values smaller than min_value.

Looking at the code it seems it is only the the value of fill_value that is used, but I'm honestly not sure of how this part is supposed to work.

maxnoe commented 11 months ago

Hi,

I think the documentation is correct and in line with the behavior, but it probably could be more clear.

The doc strings state:

    fill_value: float or quantity
        Value for bins with less than ``min_events``,
        must have same unit as values
    min_value: float or quantity or None
        If given, cuts smaller than this value are replaced with ``min_value``

So both specify a value to replace the computed value, but under different conditions:

fill_value is used when the number of events in the bin is less than the required min_events and min_value is a simple clipping value for the computed value itself.

maxnoe commented 11 months ago

The code is here:

https://github.com/cta-observatory/pyirf/blob/4b617cc678ee83dc283c6d7e5f87e8a2da566839/pyirf/cuts.py#L77-L84

It matches the documentation (fill_value used if n_events < min_events, and min/max value uses as arguments to np.clip)