first stab at floating-point comparators

[desmonddak]

Description & Motivation

This is a baseline for making arithmetic types have basic operations like comparison and negate. This provides these operations
for FloatingPoint.

Related Issue(s)

#199

Testing

All added operands have basic testing.

Backwards-compatibility

Is this a breaking change that will not be backwards-compatible? If yes, how so?

No

Documentation

Does the change require any updates to documentation? If so, where? Are they included?

No. This is an extension to the API for these types but a very natural one whose documentation is in the class itself.

[mkorbel1]

TODO: rest of the testing for the operators? we should definitely test equality right?

[desmonddak]

So LogicStructure does not support operator != or operator ==.
When I try to add, it complains in FloatingPointValue about need for non-nullable type and a hashcode.

operator != is not overridable apparently.

operator -() -- the linter cannot decide if this should have override or not!

[mkorbel1]

I never played with the - operator, interesting.

You should not override the == operator since we want that to stay as object equality. I meant we should be sure to test that eq works, and also some specific corner cases (e.g. NaN, infinities, etc.) and some of the exceptions that we're checking (mismatch widths, jbit-ness, etc.)

[desmonddak]

I'll be adding the corner case testing. Infinities are always equal (modulo the sign), but NaNs are never equal.

[desmonddak]

Should we not do this (already in the code):

floating_point_value.dart

  @override
  bool operator ==(Object other) {
    if (other is! FloatingPointValue) {
      return false;
    }
    if ((exponent.width != other.exponent.width) |
        (mantissa.width != other.mantissa.width)) {
      return false;
    }
    if (isNaN != other.isNaN) {
      return false;
    }
    if (isAnInfinity != other.isAnInfinity) {
      return false;
    }
    if (isAnInfinity) {
      return sign == other.sign;

[desmonddak]

Another weird case: I thought we could not override <= or <:

fixed_point_value.dart:

  @override
  bool operator ==(Object other) {
    if (other is! FloatingPointValue) {
      return false;
    }
    if ((exponent.width != other.exponent.width) |
        (mantissa.width != other.mantissa.width)) {
      return false;
    }
    if (isNaN != other.isNaN) {
      return false;
    }
    if (isAnInfinity != other.isAnInfinity) {
      return false;
    }
    if (isAnInfinity) {
      return sign == other.sign;

[mkorbel1]

yes we should on the *Values, but not on the things implementing Logic. The values are sort of like immutable software primitives, whereas signals are objects that change value over time during simulation.

[mkorbel1]

what lint warning? does the analyzer flag a lint or not, because it should be failing if you don't have the right ignore syntax

[desmonddak]

The analyzer flags a lint error, but it doesn't fail in CI. It is a false error, the operator works. I don't have to have this operator. I see a blue underline in my VSCode with the message that this operator needs an @override annotation

When I add override, it still complains, probably because LogicValue doesn't have a '-' operator:

[mkorbel1]

hmm weird! i wonder if it's an analyzer bug?

[mkorbel1]

resolve this comment?

also, wouldn't we always want a fixed width FixedPointValue per widths of the FloatingPointValue components rather than the width changing based on values?

[desmonddak]

The width of FixedPointValue would be fixed but very wide to make FP conversion lossless. But that width is a function of the FP width (bias * 2 + mantissaWidth + 2): the full range of sliding an FP number up and down the scale of exponents plus its mantissa plus a jbit plus a sign bit.

So I thought it made sense to do a .toFixedPointValue rather than doing an .ofFloatingPointValue since we need to compute the widths dependent on the FP exponent/mantissa widths (not values).

[desmonddak]

Maybe I didn't understand this question, but this function is dependent on FP component widths and is written to handle that -- did you see something value-based in the method?

[desmonddak]

Ah, now I see. My comment was misleading and confused. I do NOT want to build anything that is value dependent. I was considering it when I could not figure out just how wide I had to go to cover the full range.

[mkorbel1]

isn't this an infinite loop if inNumber is 0?

wouldn't null be a better return value if none is found?

also, is this a utility we want to expose in the public API? we have other things like Find which can also find the nth instance of a 1 or 0, plus an error signal on the hardware side. Maybe a more complete extension to LogicValue would be more valuable as a public API?

[desmonddak]

I like the idea of a LogicValue extension. We could mirror 'Find' in the Value side. Let me take a look.

[desmonddak]

For now I added code to match my other leading1 detectors. For some reason I have not tripped on this bug.
It makes sense to move to Find.

[sweeksBigBlue]

Are we shifting out the unbiased exponent here? I would think you would need to debias the exponent but maybe I'm missing something further here?

[desmonddak]

No, its really weird: I am using the unbiased exponent of 00...001 to be fully right-shifted mantissa and then 11....10 to be the full left-shifted mantissa stored in a really wide FixedPoint integer. We would set n/m to be around the midpoint of this super-wide LogicStructure.

[sweeksBigBlue]

Oh okay, I would for sure need that explained to me 😆

[desmonddak]

I answered poorly. yes, we are shifting the mantissa by the unbiased exponent. That way, the tiniest numbers are all the way to the right (from mantissa-1 to 0). Then the largest numbers are shifted as far left as we can so the mantissa sits at the left edge down to left edge - mantissa-width. that makes the fixed-point around 2*bias + mantissa-width wide.

[desmonddak]

I converted this PR to a draft as there is a lot to fix.