search

veeresht / CommPy

Digital Communication with Python
http://veeresht.github.com/CommPy
BSD 3-Clause "New" or "Revised" License
538 stars 176 forks source link

Fix coding rate send chunk rounding error #86

Closed eSoares closed 3 years ago

eSoares commented 3 years ago

Fixes #85

coveralls commented 3 years ago

Coverage Status

Coverage increased (+0.04%) to 79.276% when pulling 3bc2faf2062c1784cd6dc712df67aaed5a0da1ee on eSoares:fix-coding-rate-send-chunk-rounding-error into 7ae3bb836ad837de1b1a5782808f8462f0d03f6f on veeresht:master.

BastienTr commented 3 years ago

Thanks for you PR @eSoares. I didn't run your test but this make a lot of sense.

However, I'm not sure of why you are using Fraction rather than regular float? Is it just to compute the denominator that is used to determine divider?

eSoares commented 3 years ago

However, I'm not sure of why you are using Fraction rather than regular float? Is it just to compute the denominator that is used to determine divider?

Fraction helps reducing the usage of floats and keeps numbers in integers. I was noticing that every once in a while, a float like 0.75 or 0.5 when converting to fraction was getting huge numbers as numerators and denominators due the floating point errors. Letting the parameter be a Fraction completely avoids any issues that can happen with that.

BastienTr commented 3 years ago

I was noticing that every once in a while, a float like 0.75 or 0.5 when converting to fraction was getting huge numbers as numerators and denominators due the floating point errors. Letting the parameter be a Fraction completely avoids any issues that can happen with that.

I understand. However, I can't found out why you need to convert float (e.g. 0.5) to fraction. Why is a very small floating-point error an issue? Is it due to the integer division?

eSoares commented 3 years ago

I was noticing that every once in a while, a float like 0.75 or 0.5 when converting to fraction was getting huge numbers as numerators and denominators due the floating point errors. Letting the parameter be a Fraction completely avoids any issues that can happen with that.

I understand. However, I can't found out why you need to convert float (e.g. 0.5) to fraction. Why is a very small floating-point error an issue? Is it due to the integer division?

Is due to the divider calculation in https://github.com/veeresht/CommPy/pull/86/commits/c5919c2cf94ab541c7bbe3a18ef892657c9f2dd2#diff-346d73f899a78915b2bb8779e25ccc49R212 .

BastienTr commented 3 years ago

Ok and the rounding error here leads to the error that are observed in #85. If so, congratulation for spotting a so tricky bug! I'll merge this PR as soon as you confirm this.

Thanks :+1:

eSoares commented 3 years ago

The rounding error was in the send chunk size, the way was calculated didn't take in to account the coding rate.

So, if you had a coding rate of 3/4 and tried to send 600 bits with modulation of 64 QAM (6 bits per signal), it would send chunks of 600 bits. But 600 bits after that code rate is applied are in fact 800 bits to modulate, which is not divisible by 6 bits per signal! Ending sending 134 symbols, being decoded as 804 bits, after depuncturing and decoding it would cause problems, causing to always have errors in the last symbol.

Reviewing again the code, just notice a tiny error with multiple chunks per send to find the correct send chunk. Adding a commit to fix it.

BastienTr commented 3 years ago

Ok I think I get it. Thanks for providing the solution to this issue ! :smiley: