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

EventBasedMetrics documentation is misleading #19

Open
turian opened this issue Nov 3, 2021 · 0 comments
Open

EventBasedMetrics documentation is misleading #19

turian opened this issue Nov 3, 2021 · 0 comments

Comments

@turian
Copy link

turian commented Nov 3, 2021

The documentation for EventBasedMetrics is misleading.

It says:

     

t_collar : float (0,]

    Time collar used when evaluating validity of the onset and offset, in seconds. Default value 0.2

percentage_of_length : float in [0, 1]

    Second condition, percentage of the length within which the estimated offset has to be in order to be consider valid estimation. Default value 0.5

The documentation for percentage_of_length suggests that is an "AND" condition, not an "OR" condition. However, the code indicates that the max is being used, which is also what mir_eval does:

        # Detect field naming style used and validate onset
        if 'event_offset' in reference_event and 'event_offset' in estimated_event:
            annotated_length = reference_event['event_offset'] - reference_event['event_onset']

            return math.fabs(reference_event['event_offset'] - estimated_event['event_offset']) <= max(t_collar, percentage_of_length * annotated_length)

        elif 'offset' in reference_event and 'offset' in estimated_event:
            annotated_length = reference_event['offset'] - reference_event['onset']

            return math.fabs(reference_event['offset'] - estimated_event['offset']) <= max(t_collar, percentage_of_length * annotated_length)

I would suggest adapting the documentation for EventBasedMetrics based upon the mir_eval documentation for offset_ratio and offset_min_tolerance to make the sed_eval documentation clearer.

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

No branches or pull requests

1 participant