stingray icon indicating copy to clipboard operation
stingray copied to clipboard

Bugfix issue#851

Open spranav1205 opened this issue 1 year ago • 4 comments

Relevant Issue(s)/PR(s) #851

Textbook implementation of impulse response.

Edge cases are 0.5 and phase is taken into account. The response maybe zero if the width doesn't fall on the sampling times, a warning can be issued for the same.

Eg. start = 3 width = 5 dt = 4 = [0,1,0.5] start = 4 width = 4 dt = 4 = [0,0.5,0.5] start = 1 width = 3 dt = 4 = [0,0.5]

ps. the code has redundancies which I can remove once finalized

spranav1205 avatar Oct 24 '24 10:10 spranav1205

@spranav1205 thanks! A few comments: edge cases should not be plainly 0.5, but account for the exact percentage of zeros and one in a given bin. Also, I suggest implementing tests that demonstrate that the new impulse response is working correctly. I think the formulas you are using will not give the expected results!

matteobachetti avatar Oct 24 '24 10:10 matteobachetti

The version I have written (and tested :sob:) assigns one if the impulse response lies on a sampling time plainly and 0.5 at the edge. I can write one where the next node (or the previous) is assigned based on how many ones and zeros lie in the bin (essentially the area right?). Is that what we are looking for??

spranav1205 avatar Oct 26 '24 02:10 spranav1205

Hey, Sorry for the delay. I have updated the code which now considers the area in each bin instead of using 0.5 directly for edge cases.

image

In these tests, I have used dt as 4 t0 is 5, 5, 5 and 4 width is 6, 7, 8 and 8

The code considers the incomplete bins at either end and the complete bins, which are assigned one.

Am I on the right track? If so I can continue testing and refining the code.

Thanks!

spranav1205 avatar Dec 17 '24 04:12 spranav1205

Codecov Report

Attention: Patch coverage is 72.72727% with 3 lines in your changes missing coverage. Please review.

Project coverage is 82.53%. Comparing base (ba8d06d) to head (48c25c2). Report is 70 commits behind head on main.

Files with missing lines Patch % Lines
stingray/simulator/simulator.py 72.72% 3 Missing :warning:

:exclamation: There is a different number of reports uploaded between BASE (ba8d06d) and HEAD (48c25c2). Click for more details.

HEAD has 2 uploads less than BASE
Flag BASE (ba8d06d) HEAD (48c25c2)
8 6
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #855       +/-   ##
===========================================
- Coverage   95.98%   82.53%   -13.46%     
===========================================
  Files          48       48               
  Lines        9686     9731       +45     
===========================================
- Hits         9297     8031     -1266     
- Misses        389     1700     +1311     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Dec 17 '24 04:12 codecov[bot]