decimal_type_test.go: Floating points failed on aarch64-darwin

[htran-opencolo]

How fatal are these errors to ygot subcomponents?

last 10 log lines:
       >         decimal_type_test.go:278: ranges [-inf,-], [0,+]: -5.05 should be outside ranges -0.0|0.0..10.1
       >         decimal_type_test.go:278: ranges [-inf,-], [0,+]: -0.001 should be outside ranges -0.0|0.0..10.1
       >     --- FAIL: TestValidateDecimalValue/ranges_[-,-],_[0,+inf] (0.00s)
       >         decimal_type_test.go:278: ranges [-,-], [0,+inf]: -10.15 should be outside ranges -0.0|0.0..922337203685477580.8
       >         decimal_type_test.go:278: ranges [-,-], [0,+inf]: -5.0009 should be outside ranges -0.0|0.0..922337203685477580.8
       >         decimal_type_test.go:278: ranges [-,-], [0,+inf]: -0.0001 should be outside ranges -0.0|0.0..922337203685477580.8
       > E0131 16:36:09.564155   13273 util_types.go:383] unexpected type bits in yangBuiltinTypeToGoPtrType
       > FAIL
       > FAIL        github.com/openconfig/ygot/ytypes       0.358s
       > FAIL

[wenovus]

Interesting, we haven't tested ygot on ARM machines. Go is supposed to have good support for ARM so I'm not sure what's happening here. I would be willing to review an update to our GitHub actions workflow that adds validation on the ARM architecture.

[htran-opencolo]

Do you have some pointers on running GH Actions on ARM architecture? Came across a discussion using QEMU with cross-compilation here https://github.com/orgs/community/discussions/25319

I would be interested in learning more about Go ecosystem, specifically with cross-compiling.

[wenovus]

I see an action that some people are using, I think it might be the same QEMU emulation you're talking about:

• Support Linux ARM64 build images actions/runner-images#5631
• https://github.com/marketplace/actions/run-on-architecture

[robshakir]

I think this is darwin, right? I wonder whether we can use the macos-latest target to run on M1? docs.

w.r.t the initial question -- these failures will mean that decimal64 values that have ranges specified on them that use the infinity value are very likely to produce erroneous validation results. It looks like generally, the decimal64 ranges WAI, but the inf keyword is an issue.

[robshakir]

Oh, aologies -- these are not actually available it looks like. Sorry for the noise.

[dbourkey]

According to the YANG 6020 Spec decimal64 values are defined as fixed point decimal numbers.

The "fraction-digits" statement, which is a substatement to the
"type" statement, MUST be present if the type is "decimal64". It
takes as an argument an integer between 1 and 18, inclusively. It
controls the size of the minimum difference between values of a
decimal64 type, by restricting the value space to numbers that are
expressible as "i x 10^-n" where n is the fraction-digits argument.

I understand the obvious value of storing values as native Go types. However, perhaps in light of this issue, and the problems of lost resolution in floating point numbers, it would be best to model decimal64 as a fixed point number, and do all validation with fixed point number operations.

I have observed issues in existing Yang models with values being incorrectly rejected due to floating-point conversions pushing the value outside of the defined ranges for the leaf. Users may expect a level of precision that floating point numbers cannot provide.

OpenConfig/goyang defines the yang.Number type in openconfig/goyang/pkg/yang/types_builtin.go. I would propose that this type should be used instead of float64s. This would fix the issues of decimal64 tests failing on M1, and generally make the code more portable.

I would like to work on this. Are there any other considerations I should be aware of before I spend time implementing such a change and opening a PR?

[robshakir]

In practice, we have found that fixed-precision numbers are generally not what is required for the data that we are actually modelling. This approach (use of float64) seems more coherent with the actual usage [0].

The change you propose will be majorly backwards incompatible for users, such that if it is pursued, it needs to be done behind a specific flag that cannot be the default to accommodate existing users of ygot for whom this functionality has not been a problem.

[0]: in OpenConfig models, we defined an ieeefloat32 type that has generally been used since its inception for all non-integer numbers.

[dbourkey]

I understand, thank you for sharing that context. It sounds like implementing a flag for using a fixed-point model might not be worth the extra code-complexity & maintenance it brings. I will work on this for my own purposes, and will open a PR only if there is a nice clean way to do it without impacting the codebase too much.

Edit: Apologies for going off-topic, this implementation would not fix the original issue discussed here.