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

Speedup audmetric.detection_error_tradeoff() #52

Merged
merged 6 commits into from
Feb 28, 2024
Merged

Conversation

maxschmitt
Copy link
Contributor

As Detection Error Tradeoff is often used for pairwise similarities, the input similarity truth vectors can be very large.
By convering parts of the detection_error_tradeoff() function to numpy, a meaningful acceleration can be achieved (see below).
@hagenw

import api  # local copy of the modified repo audmetric.core.api
import audmetric
import numpy as np
import time


num_samples = np.array([1e4, 1e5, 1e6, 1e7]).astype(int)

for n in num_samples:
    # Generate seeded random truth and similarity vectors
    np.random.seed(0)
    truth = np.random.rand(n,) > 0.8
    sim = np.random.randn(n,) + truth * 1.

    # old implementation
    t_start = time.perf_counter()
    det1 = audmetric.detection_error_tradeoff(truth, sim)
    t_stop = time.perf_counter()
    rt1 = t_stop - t_start

    # new implementation
    t_start = time.perf_counter()
    det2 = api.detection_error_tradeoff(truth, sim)
    t_stop = time.perf_counter()
    rt2 = t_stop - t_start

    for a, b in zip(det1, det2):
        np.testing.assert_allclose(a, b)

    print(f"{n} samples - Runtime old: {rt1:.3f}s, new: {rt2:.3f}s; "
          f"ratio: {(rt2/rt1):.3f}")
   10000 samples - Runtime old:  0.012s, new: 0.002s; ratio: 0.188
  100000 samples - Runtime old:  0.154s, new: 0.026s; ratio: 0.169
 1000000 samples - Runtime old:  1.916s, new: 0.289s; ratio: 0.151
10000000 samples - Runtime old: 21.873s, new: 3.515s; ratio: 0.161

Copy link

codecov bot commented Feb 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.0%. Comparing base (2cd27f4) to head (612886d).

Additional details and impacted files
Files Coverage Δ
audmetric/core/api.py 100.0% <100.0%> (ø)

@hagenw
Copy link
Member

hagenw commented Feb 28, 2024

Great, this looks indeed much faster.

What is a little bit problematic with audmetric.detection_error_tradeoff() is that we only have the docstring test for:

>>> truth = [1, 0]
>>> prediction = [0.9, 0.1]
>>> detection_error_tradeoff(truth, prediction)
(array([1., 0.]), array([0., 0.]), array([0.1, 0.9]))

Would it be possible to add a few more cases to tests/test_api.py in a new test_detection_error_tradeoff() function, or would it be too complicated to get the expected values?

@maxschmitt
Copy link
Contributor Author

Would it be possible to add a few more cases to tests/test_api.py in a new test_detection_error_tradeoff() function, or would it be too complicated to get the expected values?

Should be fine, I will add some cases.

@maxschmitt
Copy link
Contributor Author

test_detection_error_tradeoff() has been added and I also removed scalar values from detection_error_tradeoff() and equal_error_rate() inputs as these metrics are un-defined for scalar (or single-element vectors).

Copy link
Member

@hagenw hagenw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks for adding a test.

audmetric/core/api.py Outdated Show resolved Hide resolved
tests/test_api.py Outdated Show resolved Hide resolved
@hagenw
Copy link
Member

hagenw commented Feb 28, 2024

As the pull request title will be used as the commit message when merging, it should only consist of 50 chars. I would propose to change the title of the pull request to: "Speedup audmetric.detection_error_tradeoff()"

Co-authored-by: Hagen Wierstorf <[email protected]>
@maxschmitt maxschmitt changed the title Implement speed-up by using numpy arrays from the start in detection_… Speedup audmetric.detection_error_tradeoff() Feb 28, 2024
Copy link
Member

@hagenw hagenw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, looks all good now. Please go ahead and merge.

@maxschmitt maxschmitt merged commit 0cb600c into main Feb 28, 2024
12 checks passed
@maxschmitt maxschmitt deleted the det_speedup branch February 28, 2024 12:43
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.

2 participants