Several float to int potential rounding errors/inconsistencies

[Jérémie Guichard]

Good morning/afternoon/evening all,

I've been lately trying to get subsurface tests to pass under windows. In
the current state, TestParse and TestPreferences are the two ones still
failing...

In short, the error raised by TestParse is caused by a rounding error when
converting dm4 sample depth from float meters to int millimeters. The
inconsistency was raised running the test on windows but same behavior can
be seen on linux for certain float values too. Have a look at the changes
in the pull request here for the details:
https://github.com/Subsurface-divelog/subsurface/pull/233

The fix in itself is easy, surround the booean_value * 1000 with a rint:
"rint(booean_value * 1000)" when it is assigned to the integer variable...

As I was commenting in the pull request, I have identified several other
potential similar rounding issues. The exact same one in dm5_dive code, in
maxdepth computation for both dm4_dive and dm5_dive and many other places
in parse-xml.c code and some as well (but in lesser amount) in other parts
of the code.

The one fixed here is the only one that was highlighted by a test failure
(when running TestParse on windows). How should we proceed forward?

   1. My original goal was more to get all tests passing under windows so
   it could get integrated in CI, TestPreference is still failing... So we
   could simply pull this change and get back to this issue later, focusing
   first on test execution for windows CI.
   2. Fix "blindly" all potential rounding errors, but I'm not really clear
   on what could be the side effect(s) of this.
   3. Spend time creating test cases that would highlight each rounding
   errors but the task is huge, not always straight forward (blob data), and
   current test philosophy seem to be closer to quick functional testing
   rather than deep regression testing.

What is your point of view here?


Best regards,


Jérémie
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20170305/91bdeace/attachment.html>