StingraySoftware / stingray

Anything can happen in the next half hour (including spectral timing made easy)!
https://stingray.science/stingray
MIT License
174 stars 144 forks source link

Simulator.simple_ir: width should always be > self.dt #851

Open matteobachetti opened 1 month ago

matteobachetti commented 1 month ago

Description of the Bug

The impulse response here is a simple series of ones that come after a certain delay. The user inputs a width of the series of ones. The number of ones is calculated as

        # Define constant impulse response
        h_ones = np.ones(int(width / self.dt)) * intensity

If width is smaller than self.dt, this operation gives no bins with one.

Steps/Code to Replicate the Bug

Call simple_ir with width = 0.1 * self.dt, e.g.

Expected Results

The list in ouput should be something like [0, 0, 0, 1]

Actual Results

It's [0, 0, 0]

spranav1205 commented 1 month ago

Hey! So if I understand correctly, we want to mask our impulse train (that has a frequency self.dt), with a width starting at some start time? In that case, we need to consider the phase. How about this

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

matteobachetti commented 1 month ago

@spranav1205 I guess one could just use min to avoid that the "ones" array has no elements

spranav1205 commented 1 month ago

Yes, that would certainly work. But I don't understand one thing. Say start = 1, width = 2, and self.dt = 4. The impulse should not appear while sampling, right?

Would you like me to make a PR using min?

matteobachetti commented 1 month ago

Yes, that would certainly work. But I don't understand one thing. Say start = 1, width = 2, and self.dt = 4. The impulse should not appear while sampling, right?

The impulse should always appear, having a 0 impulse response is always useless :) In this particular case, there is an additional complication, because the start is 1 which is smaller than dt as well. We can modify the method so that it also accounts for fractional contributions (e.g., the impulse response starts at 1 and lasts 2, but the dt is 4, so the impulse response will just be [0 + 2/4 + 0] = [0.5]), or otherwise decide that it just fails if the start is less than one bin, and it always have at least one bin.

e.g.

or

Would you like me to make a PR using min? Actually "max" as in the example above. I think the previous set of solutions I wrote are the best for a "simple impulse response"