ros2 / rosidl_dynamic_typesupport

Unified Interface for Dynamic (Runtime) Typesupport and Serialization
Apache License 2.0
2 stars 3 forks source link

Consider removing support for float128 (long double) #11

Open mxgrey opened 3 weeks ago

mxgrey commented 3 weeks ago

This library supports float128 (long double) fields in messages [1], presumably because the OMG IDL spec mentions them (see section 7.4.1.4.4.2.2).

However, the OMG specification leaves the exact definition of a long double ambiguous: "The long double data type represents an IEEE double-extended floating-point number, which has an exponent of at least 15 bits in length and a signed fraction of at least 64 bit" (emphasis mine). It is important to note that this ambiguity means that the binary representation of a long double is not generally safe to transmit across applications since the applications may be generated by different compilers or against different platforms which may have incompatible binary representations of a long double, leading to invalid data if they interpret the format wrongly.

The IEEE extended floating point formats were conceived for performing internal computations at higher precision to minimize round-off errors, which is why their specification is ambiguous: They were meant to be invisible to programmers, not serialized and transmitted, so it was not considered important to be firm about their definition.

Presumably OMG IDL includes long double for the sake of completeness, in case some user has some reason for serializing and moving intermediary calculation cache data around within their application, but this seems like an extremely caveat emptor situation.

Since a key goal of ROS is to be accessible to users who are not experts in computing or binary representations, free of the landmines and gotchyas that come with ambiguous specifications, I think it would be reasonable to argue that we should not consider the movement of intermediary extended floating point calculations within a single application to be a valid use case for ROS. The only way that transporting a long double can be safe is if we also encode and enforce the binary representation, and we do not currently have a reasonable way to do that.

In terms of support across different middlewares:

While the ROS IDL design docs mention support for long double, it has never made it into the builtin types list for ROS IDL, so there is currently no way for ROS users to transmit long doubles anyway outside of this dynamic typesupport library.

I think long double is a massive footgun for ROS users that adds no tangible value, so for the sake of safety and soundness I propose that we eliminate its presence from this dynamic type support library, and also remove it from the "Supported subset of IDL" list in the ROS IDL design doc.

clalancette commented 3 weeks ago

Thanks for laying out the issue so well. I also took a look around the ROS ecosystem (at least, all of the packages released into Rolling), and as far as I can tell nothing expects to be able to transmit float128 on the wire currently.

I'm going to bring this up at the next ROS PMC meeting to see if anyone has objections or additional thoughts about removing this support.

MiguelCompany commented 2 weeks ago

@clalancette I will not be able to attend the PMC meeting, but I have no objections about removing support for float128.