Repository: pyfar/pyfar (python)

Issue:
Find IR delay - Polyfit Error
I was using the function `pyfar.dsp.find_impulse_response_delay()` and stumbled upon an error that I cannot seem to pinpoint exactly. The function works fine for impulse responses that are very similar to the one I attached to this issue, but for this one, the polynomial fit fails. Maybe you can understand why exactly this IR seems to raise an issue.

Minimal code example
------
```python
import pyfar as pf
import os

s = pf.io.read_audio(os.path.join(os.path.dirname(__file__), "ir_problem.flac"))
pf.dsp.find_impulse_response_delay(s)
```

Error message
---------
File "check_ir.py", line 5, in <module>
    pf.dsp.find_impulse_response_delay(s)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^
  File "/opt/miniconda3/envs/playground/lib/python3.13/site-packages/pyfar/dsp/dsp.py", line 1325, in find_impulse_response_delay
    search_region_poly = np.polyfit(
        search_region_range[mask]-argmax, search_region[mask], N)
  File "/opt/miniconda3/envs/playground/lib/python3.13/site-packages/numpy/lib/_polynomial_impl.py", line 636, in polyfit
    raise TypeError("expected non-empty vector for x")
TypeError: expected non-empty vector for x

Environment
--------
Python: 3.13.1
pyfar: 0.7.1
Macbook Pro M1

[ir_problem.flac.zip](https://github.com/user-attachments/files/18966566/ir_problem.flac.zip)

Pull request description:
hot fix for crash in dsp.find_impulse_response_delay
### Which issue(s) are closed by this pull request?

Closes #775 

### Changes proposed in this pull request:

- Add if case for catching the bug
- Add test for the critical case.

The new test failed before changing `find_impulse_response_dealy`, indicating that the suggested change fixes the bug.


Patch:
diff --git a/pyfar/dsp/dsp.py b/pyfar/dsp/dsp.py
index 0aabd899..3ab8ee07 100644
--- a/pyfar/dsp/dsp.py
+++ b/pyfar/dsp/dsp.py
@@ -1292,7 +1292,8 @@ def find_impulse_response_delay(impulse_response, N=1):
             ir = impulse_response.time[ch]
             ir = np.real(ir) if mode == 'real' else np.imag(ir)
 
-            if np.max(ir) > 1e-16:
+            #Check absolute amximum, because peaks can be positive or negative.
+            if np.max(np.abs(ir)) > 1e-16:
                 # minimum phase warns if the input signal is not symmetric,
                 # which is not critical for this application
                 with warnings.catch_warnings():
@@ -1318,6 +1319,16 @@ def find_impulse_response_delay(impulse_response, N=1):
                 search_region = np.imag(
                     correlation_analytic[search_region_range])
 
+                # If this is true, it indicates that `correlation_analytic` has
+                # a negative peak, which can happen if the absolute maximum of
+                # `impulse_response` is negative. Changing the sign of the
+                # search region makes sure that the `mask` generated below
+                # works as intended. Fixing it this way is safer because it is
+                # theoretically possible that the absolute maximum of
+                # `impulse_response` is negative but
+                # `correlation_analytic[argmax].real` is positive.
+                search_region *= np.sign(correlation_analytic[argmax].real)
+
                 # mask values with a negative gradient
                 mask = np.gradient(search_region, search_region_range) > 0
 


Test patch:
diff --git a/tests/test_dsp.py b/tests/test_dsp.py
index e8a18f4d..98715054 100644
--- a/tests/test_dsp.py
+++ b/tests/test_dsp.py
@@ -764,7 +764,8 @@ def test_impulse_response_delay(is_complex):
     npt.assert_allclose(start_sample_est, start_sample, atol=1e-2)
 
 
-def test_impulse_response_delay_sinc():
+@pytest.mark.parametrize('sign', [1, -1])
+def test_impulse_response_delay_sinc(sign):
     """Test delay of a band-limited sinc function shifted by 1/2 samples."""
     sr = 44100
     n_samples = 128
@@ -774,6 +775,10 @@ def test_impulse_response_delay_sinc():
     sinc = np.sinc(samples - delay_samples)
     win = sgn.get_window('hann', n_samples, fftbins=False)
 
+    # Impulse responses with large negative peaks require special handling in
+    # `find_impulse_response_delay`. This makes tests the special case.
+    sinc *= sign
+
     ir = pf.Signal(sinc*win, sr)
     start_samples = pf.dsp.find_impulse_response_delay(ir)
     npt.assert_allclose(start_samples, delay_samples, atol=1e-3, rtol=1e-4)