Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make casting negative numbers more predictable. #96

Open
different55 opened this issue Aug 5, 2021 · 2 comments
Open

Make casting negative numbers more predictable. #96

different55 opened this issue Aug 5, 2021 · 2 comments
Assignees
Labels
Bug Report This issue reports a bug

Comments

@different55
Copy link

Brief Description

Casting between different signed types can have unintuitive (incorrect?) results. Some examples of how to do this correctly would be welcomed.

Observed Behaviour

When casting negative Q1.14 values to Q3.12, the results are very unexpected. The fractional bit of the number seems to be within ~1 of the true value, but the integer part can be quite a bit off. What should be -1 ends up being 3. Or positive 2 instead of -2.

Expected Behaviour

Casting a fixed point number from one format to another should maintain a roughly equivalent value.

Source Code

#include <FixedPoints.h>

void setup() {
	Serial.begin(115200);
	
	SFixed<1,14> a = -0.05;
	SFixed<3, 12> b = static_cast<SFixed<3, 12>>(a);
	SFixed<3, 12> c = -0.05;
	Serial.print("Initial Q1.14 value: "); Serial.println((float)a);
	Serial.print("Value cast to Q3.12: "); Serial.println((float)b);
	
	Serial.println();
	
	Serial.print("Internal representation of Q1.14: "); Serial.println(a.getInternal(), BIN);
	Serial.print("Expected representation of Q3.12: "); Serial.println(c.getInternal(), BIN);
	Serial.print("Actual representation of Q3.12: "); Serial.println(b.getInternal(), BIN);
	
	Serial.println();
	
	Serial.print("Q1.14 integer bit: "); Serial.println(a.getInteger());
	Serial.print("Q1.14 fraction bit: "); Serial.println(a.getFraction());
	Serial.print("Wonky Q3.12 integer bit: "); Serial.println(b.getInteger());
	Serial.print("Wonky Q3.12 fraction bit: "); Serial.println(b.getFraction());
	Serial.print("Correct Q3.12 integer bit: "); Serial.println(c.getInteger());
	Serial.print("Correct Q3.12 fraction bit: "); Serial.println(c.getFraction());
}

void loop() {}

Output:

Initial Q1.14 value: -0.05
Value cast to Q3.12: 3.95

Internal representation of Q1.14: 11111111111111111111110011001101
Expected representation of Q3.12: 11111111111111111111111100110100
Actual representation of Q3.12: 11111100110011

Q1.14 integer bit: -1
Q1.14 fraction bit: 15565
Wonky Q3.12 integer bit: 3
Wonky Q3.12 fraction bit: 3891
Correct Q3.12 integer bit: -1
Correct Q3.12 fraction bit: 3892

Extra Information

  • Target Board: Arduino Uno (R3?)
  • Compiler Version: avr-g++ 7.3.0
  • Arduino IDE Version: 1.8.10
  • OS: Debian Sid
@different55 different55 added the Bug Report This issue reports a bug label Aug 5, 2021
@Pharap
Copy link
Owner

Pharap commented Aug 5, 2021

To cut a long story short, it's because internally the conversion is using unsigned shift operations (i.e. 'logical' shifting is being forced rather than 'arithmetic' shifting).

I can't remember whether this was intentional or not, but it quite possibly was because technically signed right shift is 'implementation defined' prior to C++20, whilst unsigned shifting is well defined.

The fix would be fairly simple, it would just be a matter of removing a few casts and consequently some unneeded local type aliases.

However, even with that the result won't be quite perfect because of the inherant loss of precision involved.

@Pharap
Copy link
Owner

Pharap commented Sep 25, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Report This issue reports a bug
Projects
None yet
Development

No branches or pull requests

2 participants