Skip to content

Fix misleading statement regarding sign bit - #260

Merged
kimikage merged 2 commits into
JuliaMath:masterfrom
ettersi:patch-1
Oct 16, 2026
Merged

kimikage merged 2 commits into
JuliaMath:masterfrom
ettersi:patch-1

Conversation

@ettersi

@ettersi ettersi commented Nov 10, 2022

Copy link
Copy Markdown
Contributor

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.

@codecov

codecov Bot commented Nov 10, 2022 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.22%. Comparing base (6decae1) to head (3ea0ce4).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kimikage

kimikage commented Apr 5, 2024

Copy link
Copy Markdown
Collaborator

I know this is a matter of taste, but I think the changed description focuses too much on the internal implementation.
In extreme terms, whether or not 2's complement is used is irrelevant to the interface of Fixed (except for reinterpret).
Also, the "decimal" point may be introducing new confusion.

Perhaps what we need is more graphical documentation (with Documenter.jl).

@ettersi

ettersi commented Apr 6, 2024 •

Copy link
Copy Markdown
Contributor Author

The main problem I see with the current description is that to me, "1 sign bit, f fraction bits" suggests a representation of the form sign * unsigned_integer / 2^f. So in particular, based on that description I would assume that the smallest (most negative) Fixed{IntN,f} is -(2^(N-1)+1) / 2^f, that there is a positive and negative zero like for floating-point numbers, and I wouldn't expect the overflow behavior that Fixed{T,f} inherits from the underlying integer type.

@kimikage

kimikage commented Apr 6, 2024 •

Copy link
Copy Markdown
Collaborator

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".
However, we have difficulty rationalizing the naming rule for aliases such as Q0f7 without using "sign bit".

I simply wanted to maintain this PR since it has been left open. So, I will leave this open.

@PatrickHaecker

PatrickHaecker commented Aug 1, 2024 •

Copy link
Copy Markdown
Collaborator

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.

I think the specific example work well enough to eliminate such ambiguity.

This example didn't help me, although I now see in retrospect how it could have helped.

What helped me, was this example

Fixed{Int8,1}(0) - eps(Fixed{Int8,1}) |> reinterpret |> (x -> reinterpret(UInt8, x))

returning 0xff.

ettersi and others added 2 commits October 16, 2026 10:36
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.
@PatrickHaecker

Copy link
Copy Markdown
Collaborator

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.

@kimikage

Copy link
Copy Markdown
Collaborator

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.
(As for the mention of "overflow", it strikes me as somewhat misleading.)

However, I believe it is better to update the document and move on rather than leave this PR open.

Thanks!

@kimikage
kimikage merged commit a9e0ed3 into JuliaMath:master Oct 16, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants