-
Notifications
You must be signed in to change notification settings - Fork 38
Floating Point to Fixed and Fixed to Floating Point Conversion #242
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
base: main
Are you sure you want to change the base?
Changes from all commits
e9ed100
62dd49b
fe901ab
ae26223
c0fbb9c
ca9ac13
8ab25ea
552a1c3
b1f3250
fc6d0f2
b41af80
5ab6019
c4c4eeb
6c94c7f
ce13380
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -291,4 +291,36 @@ void main() { | |
| } | ||
| } | ||
| }); | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about testing with vs without
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you mind expanding on that? Idk what that is but happy to add it to my tests.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://github.com/intel/rohd-hcl/blob/main/doc/components/floating_point.md#explicit-j-bit I'll test the documentation on you: if this doc doesn't explain it well enough, we should improve it!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great question! I think maybe this would be an excellent second issue I can follow-up on in a different PR maybe? Honest answer is, even with reading that I'd need time to digest that and maybe that should be an expanded method to this base one? Or maybe I'm over complicating that.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it should be a relatively straight-forward change, basically even normal things look like sub-normals with an explicit
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry for the late reply: it should be quite simple; a small change in how you adjust the mantissa in each direction. But going to an explicit j-bit form, you would keep that leading 1 and put it into the mantissa. The FPV feels effectively one bit shorter because you need to store the leading '1. Note that you would do this just as you do with subnormals. In other words, if you fixed point was small enough to require a subnormal representation in FPV, you would also keep that leading '1'. Going from an explicitJBit normal float to fixed, you would recognize that you don't need to prefix the mantissa with a '1' representing the j-bit. The leading '1' is already in the mantissa. This is just like it is for subnormal. If you are converting a subnormal float to fixed, you do not prefix the mantissa with a '1'. To test, you need to provide an explicitJBit form of floatingPointValue to translate to fixed and conversely, indicate that you want an explicitJBit form of floatingPointValue when converting from fixed. Explicit JBIT is experimental: we recognize that we do not have to normalize in some situations as it doesn't produce any more accuracy (say adding two narrow mantissa FPs to get a wider mantissa FP). Rather than normalizing, we can leave it in explicit j-bit form for the next operation and save a normalization step, and let normalization happen after the next operation.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Something like this;
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jcfarwe has pushed tests for J-bit, ready for review @desmonddak and @mkorbel1
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tests look good. Can you merge the signed/unsigned versions into one?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you shorten the widths to speed up the tests? 22 bits is quite wide. I tried the following which ran in about 1s: for (final (m, n) in [(5, 7), (7, 5), (0, 7), (7, 0)]) { |
||
| test('FixedPointValue: toFloatingPointValue signed', () { | ||
| for (final (m, n) in [(10, 12), (0, 22), (22, 0)]) { | ||
| final width = m + n + 1; // 1 for sign bit | ||
| for (final explicitJBit in [true, false]) { | ||
| for (var i = 0; i < pow(2, width); i++) { | ||
| final fxv = FixedPointValue.populator( | ||
| integerWidth: m, fractionWidth: n, signed: true) | ||
| .ofLogicValue(LogicValue.ofInt(i, width)); | ||
| final fpv = fxv.toFloatingPointValue(explicitJBit: explicitJBit); | ||
| expect(fpv.toDouble(), fxv.toDouble(), | ||
| reason: 'toFloatingPointValue failed for $i'); | ||
| } | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| test('FixedPointValue: toFloatingPointValue unsigned', () { | ||
|
desmonddak marked this conversation as resolved.
|
||
| for (final (m, n) in [(10, 12), (0, 22), (22, 0)]) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Definitely narrow these widths... |
||
| final width = m + n; // no sign bit for unsigned | ||
| for (final explicitJBit in [true, false]) { | ||
| for (var i = 0; i < pow(2, width); i++) { | ||
| final fxv = | ||
| FixedPointValue.populator(integerWidth: m, fractionWidth: n) | ||
| .ofLogicValue(LogicValue.ofInt(i, width)); | ||
| final fpv = fxv.toFloatingPointValue(explicitJBit: explicitJBit); | ||
| expect(fpv.toDouble(), fxv.toDouble(), | ||
| reason: 'toFloatingPointValue failed for $i'); | ||
| } | ||
| } | ||
| } | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -631,4 +631,152 @@ void main() { | |
| } | ||
| }); | ||
| }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We like to see a blank line between tests or groups. |
||
| group('FPV: toFixedPointValue', () { | ||
| // generate expected result of float2fixed conversion | ||
| FixedPointValue expectedResult( | ||
| int exp, | ||
| int expSize, | ||
| int sign, | ||
| int mant, | ||
| int mantSize, | ||
| LogicValue exponent, | ||
| ) { | ||
| // generate expected result | ||
| final expAbs = exp.abs(); | ||
| final shift = | ||
| expAbs + 3; // add two bits for integral part, one bit for sign | ||
|
|
||
| final mantissa = exponent != LogicValue.ofInt(0, expSize) | ||
| ? LogicValue.ofInt(1 << mantSize | mant, mantSize + shift) | ||
| : LogicValue.ofInt(mant, mantSize + shift); | ||
| final shiftedMantissa = exp < 0 ? mantissa : mantissa << expAbs; | ||
| final finalMantissa = sign == 0 ? shiftedMantissa : ~shiftedMantissa + 1; | ||
|
|
||
| final nLen = exp.isNegative ? mantSize - exp : mantSize; | ||
| final mLen = finalMantissa.width - nLen - 1; // one bit for sign | ||
| return FixedPointValue.populator( | ||
| integerWidth: mLen, | ||
| fractionWidth: nLen, | ||
| signed: true, | ||
| ).ofLogicValue(finalMantissa); | ||
| } | ||
|
|
||
| test('FPV: toFixedPointValue exhaustive', () async { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Something is fishy about this test. It only fills the upper 20 bits or so of the integer portion, yet the test is labeled 'exhaustive'. Here is what I see when I print |
||
| // bit widths to be tested | ||
| // 5, 6, 7, 8-bit exponent | ||
| // 10, 12, 17, 23-bit mantissa | ||
| final expWidths = [5, 6, 7, 8]; | ||
| final mantWidths = [10, 12, 17, 23]; | ||
|
|
||
| for (final expWidth in expWidths) { | ||
| for (final mantWidth in mantWidths) { | ||
| final minExp = -1 << (expWidth - 1); | ||
| final maxExp = (1 << (expWidth - 1)) - 1; | ||
| for (var testExp = minExp; testExp < maxExp; testExp++) { | ||
| for (final sign in [LogicValue.zero, LogicValue.one]) { | ||
| final bias = (pow(2, expWidth - 1) - 1).toInt(); | ||
| final exp = LogicValue.ofInt(testExp, expWidth); | ||
| final mant = LogicValue.ofInt(testExp, mantWidth); | ||
|
|
||
| final fpv1 = FloatingPointValue( | ||
| exponent: exp + bias, | ||
| sign: sign, | ||
| mantissa: mant, | ||
| ); | ||
| if (fpv1.isNaN || fpv1.isAnInfinity) { | ||
| continue; | ||
| } | ||
|
|
||
| final fxv = fpv1.toFixedPointValue(); | ||
|
|
||
| final expected = expectedResult( | ||
| testExp, | ||
| expWidth, | ||
| sign.toInt(), | ||
| mant.toInt(), | ||
| mantWidth, | ||
| fpv1.exponent, | ||
| ); | ||
|
|
||
| expect( | ||
| fxv == expected, | ||
| true, | ||
| reason: 'Got $fxv expected $expected', | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }); | ||
| test('FPV: toFixedPointValue simplified', () async { | ||
| // | ||
| //(exp, expSize, sign (0 == +), mant, mantSize) | ||
| final testCases = [ | ||
| (0, 8, 0, 0x000000, 23), // 1.0 | ||
| (0, 8, 0, 0x000001, 23), // 1.0000001 | ||
| (1, 8, 0, 0x000001, 23), // 2.0000002 | ||
| (8, 8, 0, 0x000001, 23), // 256.00003 | ||
| (0, 8, 0, 0x400000, 23), // 1.5 | ||
| (-1, 8, 0, 0x400000, 23), // 0.75 | ||
| (1, 8, 0, 0x400000, 23), // 3.0 | ||
| (2, 8, 0, 0x400000, 23), // 6.0 | ||
| (0, 8, 1, 0x000000, 23), // -1.0 | ||
| (0, 8, 1, 0x400000, 23), // -1.5 | ||
| (0, 5, 0, 0x000, 10), // 1.0, 16-bit float | ||
| (-127, 8, 0, 0x000001, 23), // 1e-45 | ||
| (-127, 8, 0, 0x000000, 23), // 0 | ||
| (-127, 8, 1, 0x000000, 23), // -0 | ||
| ]; | ||
|
|
||
| for (final testCase in testCases) { | ||
| final exp = testCase.$1; | ||
| final expSize = testCase.$2; | ||
| final bias = (pow(2, expSize - 1) - 1).toInt(); | ||
| final sign = testCase.$3; | ||
| final mant = testCase.$4; | ||
| final mantSize = testCase.$5; | ||
|
|
||
| final fpv1 = FloatingPointValue( | ||
| exponent: LogicValue.ofInt(exp + bias, expSize), | ||
| sign: LogicValue.ofInt(sign, 1), | ||
| mantissa: LogicValue.ofInt(mant, mantSize), | ||
| ); | ||
| final fxv = fpv1.toFixedPointValue(); | ||
|
|
||
| // generate expected result | ||
| final expected = expectedResult( | ||
| exp, | ||
| expSize, | ||
| sign, | ||
| mant, | ||
| mantSize, | ||
| fpv1.exponent, | ||
| ); | ||
|
|
||
| expect(fxv == expected, true, reason: 'Got $fxv expected $expected'); | ||
| } | ||
| }); | ||
| }); | ||
| test('FPV: toFixedPointValue, Special values', () async { | ||
| // | ||
| //[exp, expSize, sign (0 == +), mant, mantSize] | ||
| final testCases = [ | ||
| [128, 8, 0, 0x400000, 23], // NaN | ||
| [128, 8, 1, 0x000000, 23], // -Inf | ||
| [128, 8, 0, 0x400000, 23], // Inf | ||
| ]; | ||
|
|
||
| for (final testCase in testCases) { | ||
| final bias = (pow(2, testCase[1] - 1) - 1).toInt(); | ||
| final fpv1 = FloatingPointValue( | ||
| exponent: LogicValue.ofInt(testCase[0] + bias, testCase[1]), | ||
| sign: LogicValue.ofInt(testCase[2], 1), | ||
| mantissa: LogicValue.ofInt(testCase[3], testCase[4])); | ||
|
|
||
| expect( | ||
| fpv1.toFixedPointValue, | ||
| throwsA(isA<RohdHclException>()), | ||
| ); | ||
| } | ||
| }); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.