Repository navigation
Fix misleading statement regarding sign bit - #260
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #260 +/- ##
=======================================
Coverage 97.22% 97.22%
=======================================
Files 7 7
Lines 792 792
=======================================
Hits 770 770
Misses 22 22 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
I know this is a matter of taste, but I think the changed description focuses too much on the internal implementation. Perhaps what we need is more graphical documentation (with Documenter.jl). |
|
The main problem I see with the current description is that to me, "1 sign bit, |
|
I believe I understand your thinking, and I think the specific example work well enough to eliminate such ambiguity. -1.0 = -128/128 ≤ x ≤ 127/128 ≈ 0.992.As far as the section is concerned, it is reasonable not to refer to the msb as "sign bit". I simply wanted to maintain this PR since it has been left open. So, I will leave this open. |
|
The "1 sign bit" also mislead me. It does not need to get as technical as proposed here, but I think the current wording is plain wrong.
This example didn't help me, although I now see in retrospect how it could have helped. What helped me, was this example returning |
The original formulation of the README made me think that the bit pattern of `Fixed{IntN,f}` numbers is `[sign bit] [Int(N-1) number]`. I hope this alternative formulation makes it clearer that `Fixed{T,f}` numbers are based on 2's complement just like the underlying integer type `T`.
"1 bit for sign, `f` bits for the fraction" suggests a sign-magnitude layout with a negative zero and a symmetric range. Define the value as the raw integer divided by `2^f` instead, and show that raw `-1` is the negative number closest to zero. Name the Q notation variant behind the `QXfY` aliases, because it explains why `X+Y` is one less than the number of bits.
e571dd4 to
3ea0ce4
Compare
|
I included (hopefully) everything which was discussed into the PR and rebased it. I hope this should cover everyone's position and clarify the issue without being too verbose. |
|
I am not a native English speaker, and since I am already familiar with the internal implementation of this package, I cannot fairly judge what would be easy for most people to understand. However, I believe it is better to update the document and move on rather than leave this PR open. Thanks! |
The original formulation of the README made me think that the bit pattern of
Fixed{IntN,f}numbers is[sign bit] [Int(N-1) number]. I hope this alternative formulation makes it clearer thatFixed{T,f}numbers are based on 2's complement just like the underlying integer typeT.