lat, lng force math operations

[qbazd]

lost precision, probably, wrong automatic casts or something. Still differs from NMEA processing on PC (pynmea2) but its fractions of mm

[mikalhart]

While I appreciate the thought behind this pull request, I'm not convinced that it's necessary.

I did an experiment with millions of random values on an Arduino Nano, comparing the original expression to the revised one and there never was any difference. Is there a platform that you can point me to where the values DO differ? It seems like they should be the same.

double ret = rawLatData.deg + rawLatData.billionths / 1000000000.0;

Here, per C++ standards, the constant 1000000000.0 is already of type "double", so there is no need to cast it. That means that rawLatData.billionths is implicitly cast to "double" before the division is performed. Then, because the result of the division is again "double", rawLatData.deg is ALSO implicitly cast, yielding the "double" result.

Comments? Push back?

[qbazd]

Thanks for appreciation.

My original issue was that I did logger based on Teensy 3.5 and logged whole NMEA sentences and data processed with TinyGPSPlus in binary form. After comparison of data NMEA log converted to .CSV by pynmea2 or Quantum GIS or some values "by hand". I had an issue with precision. Long story short this solved the problem. So I've shared my solution.

I totally agree with an explanation that compiler "should" deal with the job. But when it comes to "should" I always try to force compiler. It looks terrible - I know - but I don't argue with it when it works. This could be the issue with my compiler, my version of compiler or else or maybe the rawLatData.deg was not implicitly cased and rawLatData.billionths / 1000000000.0 was down cast before and cast again to double. I don't know.

As I do lot's of work involving precise GPS logging and GIS processing for science and I can for 100% say it was real issue.

I could try to do the test case for this issue, but I moved from NMEA to binary Ublox protocol so I'm not ready right now. And also moved from AVR to ARM.

But the test case would be:

1. Generating NMEA stream in arduino code.
2. Process it with TinyGPS
3. Write to serial generated NMEA stream and processed position in binary form (as printf under Arduino could have issues with the double).
4. Do comparison of both on PC.

What do you think?

[mikalhart]

Still not clear why this would be needed, but @qbazd analysis seems plausible...?

[svdrummer]

I changed my notation in my defines some time ago. Depending on the compiler used, some generic terms are compiled differently depending on which micro is used. See below: Description. Double precision floating point number. On the Uno and other ATMEGA based boards, this occupies 4 bytes. That is, the double implementation is exactly the same as the float, with no gain in precision. On the Arduino Due, doubles have 8-byte (64 bit) precision.

ESP32 boards use 64.

I found (By reading what others had found) that writing code using INT, DOUBLE etc would change in output depending on the chip i was using at the time. If I used uint8_t and uint16_t, uint32_t i was in fact TELLING the compiler how to define the variable. What go me was, some compilers treat an INT as 32 bits and others use 16 bits.

I looked more into this about the time tinyGPS++ switched LAT and LONG from float to double.
This is when i stumbled upon compiler issues not behaving the same.... which explained why the same code behaved differently depending which board i was using.

My point is, don't assume (like I did) that compilers stick to a standard.

[TD-er]

Then I wonder what your compiler would make of this:

double ret = (static_cast<double>(rawLatData.billionths) / 1000000000.0) + rawLatData.deg;

I think that's as basic as it gets and in principle the static_cast shouldn't be needed, but just for good measure. The reason why this may differ is that there is indeed a difference in how compilers consider what to cast to before performing the operations. So using this notation there is no doubt the + operator for a double should be used.

Another approach may be to do it like this:

double ret = static_cast<double>(rawLatData.deg) + (rawLatData.billionths) / 1000000000.0);

As I mentioned the 1000000000.0 without the trailing f should already be a double.

And when really in doubt, use this:

double ret = static_cast<double>(rawLatData.deg) + (static_cast<double>(rawLatData.billionths)) / 1000000000.0);

But using a C-style cast (double)foo is asking for odd side-effects as some (all?) compilers may treat it as a reinterpret_cast and it may cause all kinds of strange issues when you move from a smaller type to a larger type (in terms of sizeof())

[qbazd]

Hi,

I can add that I used Teensy 3.6 board back then and I built GPS logger for Antarctic for scientists.

I think proper tests incorporated into the library would resolve the data resolution issue. Eg. parsing of known NMEA stream, should give known floating output with some margin of error.

As you all see the are many factors to take into account (some of witch I'm guessing):

• platform specific eg. (double is implemented as float) maybe assert(sizeof(double) == 8) ?
• compilation flags ?
• software/hardware floating implementations
• compiler optimisations
• code interpretation by compiler (I understand standards etc but reality is sometimes different)

maybe the float64_t would be explicit: https://stackoverflow.com/a/76201716

All the best, Jakub

niedz., 12 lis 2023 o 01:40 svdrummer @.***> napisał(a):

I changed my notation in my defines some time ago. Depending on the compiler used, some generic terms are compiled differently depending on which micro is used. See below: Description. Double precision floating point number. On the Uno and other ATMEGA based boards, this occupies 4 bytes. That is, the double implementation is exactly the same as the float, with no gain in precision. On the Arduino Due, doubles have 8-byte (64 bit) precision.

ESP32 boards use 64.

I found (By reading what others had found) that writing code using INT, DOUBLE etc would change in output depending on the chip i was using at the time. If I used uint_8 and uint_16, uint_32 i was in fact TELLING the compiler how to define the variable. What go me was, some compilers treat an INT as 32 bits and others use 16 bits.

I looked more into this about the time tinyGPS++ switched LAT and LONG from float to double. This is when i stumbled upon compiler issues not behaving the same.... which explained why the same code behaved differently depending which board i was using.

— Reply to this email directly, view it on GitHub https://github.com/mikalhart/TinyGPSPlus/pull/47#issuecomment-1806960356, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABH4OKAT7IDVUJDWK3O53OTYEALHXAVCNFSM4FGTJKU2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOBQGY4TMMBTGU3A . You are receiving this because you were mentioned.Message ID: @.***>

[TD-er]

I never heard of a std::float64_t Not sure if that will be the best solution as it will probably add a lot of code to implement something that's already been implemented as double. Since this is a library, I guess it would then add quite a bit to the compiled binary size as users may be using both types in their code.

If it is just a simple wrapper for platforms already supporting double, then the added code would probably be negligable.

[mikalhart]

I feel the strategies mentioned here, like exploring alternate types (float64_t), or looking at compilation flags, or sizeof(double) vs sizeof(float), etc., don't address the primary issue, which is the claim that the expressions

double ret = rawLatData.deg + rawLatData.billionths / 1000000000.0; and double ret = (double)rawLatData.deg + (double)rawLatData.billionths / (double)1000000000.0;

yield different results in at least one circumstance/platform/scenario. I gently challenge this claim and (respectfully) ask the OP to provide a reproducible demonstration before we expend too much time writing test cases to try to prove that they don't, or solving a problem that doesn't exist.

(Note: I don't dispute that the calculated 'ret' values may differ between two different compilers/platforms. But on the same one, I claim they are identical.)

[svdrummer]

I am happy with the tinyGPS precision, as it is as accurate as the standard GPS NEO6,7,8 modules. For what is use them for, i am more interested in statis drift I only mention the Uint32_t as i got caught with compilers not being the same.

[mikalhart]

Closing this PR until we can get a reproducible demonstration of an instance where it is needed.