Skip to content

Conversation

@kashish2210
Copy link
Member

@kashish2210 kashish2210 commented Mar 4, 2025

Bug Fix for Issue #851: Simulator.simple_ir: width should always be > self.dt

Description of the Bug

The simple_ir function constructs an impulse response by appending a series of ones after a certain delay. However, when the width parameter is smaller than self.dt, the calculation:

h_ones = np.ones(int(width / self.dt)) * intensity

results in an empty array (h_ones = []). This leads to an incorrect output where the expected impulse response does not contain the required 1 value.

Steps to Replicate the Bug

  1. Create an instance of the class containing simple_ir.
  2. Call the function with width smaller than self.dt, for example:
    width = 0.1 * self.dt
    impulse_response = sim.simple_ir(start=3, width=width, intensity=1)
    print(impulse_response)
  3. Observe the output:
    Actual Output: [0, 0, 0]
    Expected Output: [0, 0, 0, 1]

Proposed Fix

To ensure that at least one bin is always created, we modify the h_ones calculation to:

num_bins = max(int(width / self.dt), 1)
h_ones = np.ones(num_bins, dtype=int) * intensity

How This Fix Works

  • Ensures at least one bin is created in the impulse response when width < self.dt.
  • Prevents an empty h_ones array, which was the root cause of the issue.
  • Ensures integer output (dtype=int) to match expected results [0, 0, 0, 1] instead of [0. 0. 0. 1.].

Testing the Fix

Run the following test case:

width = 0.1 * sim.dt
impulse_response = sim.simple_ir(start=3, width=width, intensity=1)
print(impulse_response)  # Expected output: [0, 0, 0, 1]

Before Fix: [0, 0, 0] (Buggy output)

After Fix: [0, 0, 0, 1] (Correct output)

Pull Request Guidelines Followed

  • Followed coding standards (PEP8, NumPy conventions).
  • Tested the fix with a valid test case.

Preview Output:

WhatsApp Image 2025-03-04 at 15 08 11_e08a065cWhatsApp Image 2025-03-04 at 15 03 22_5927f061

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.

1 participant