Skip to content

Lead to inaccurate comparison, using EPSILON to correct.#350

Closed
devnexen wants to merge 1 commit into
nmap:masterfrom
devnexen:master
Closed

Lead to inaccurate comparison, using EPSILON to correct.#350
devnexen wants to merge 1 commit into
nmap:masterfrom
devnexen:master

Conversation

@devnexen

Copy link
Copy Markdown

No description provided.

Comment thread timing.cc
else
interval = TIMEVAL_SUBTRACT(*now, start_tv) / 1000000.0;
assert(diff <= interval);
assert(diff <= interval + std::numeric_limits<double>::epsilon());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After investigation it seems like this is not an accurate method for comparaison between doubles less than 1.0, and it is possible to obtain interval == 0.0 so it may not be accurate enough for this comparaison. At https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/, the author explains how float(pi)+sin(float(pi)) is more accurate than float(pi) to pi. I would suggest to compare double(interval)+sin(double(interval)) to double(diff)+sin(double(diff))... but I didn't check if the results would be in fact more accurate. What do you think?

@ghost

ghost commented Jul 16, 2016

Copy link
Copy Markdown

Hello @devnexen, after some tests, it seems like your suggestion is the most accurate one among a few others. I will thus publish your commit soon by crediting it to you. Talk to you soon and thank you for the contribution!

@dmiller-nmap

Copy link
Copy Markdown

@W0naN0w Would you mind publishing as a gist and linking here the test code you used to determine what methods were accurate for comparison here?

@ghost

ghost commented Jul 20, 2016

Copy link
Copy Markdown

@dmiller-nmap Of course! Sorry for yesterday I had a rough day. Here is the test I used to come to this conclusion: Gist link. You can find the output of the tests at the same place: Gist output link.
I hope it's what you were looking for, because I don't really know how to make a clearer test.

@nmap-bot nmap-bot closed this in c104245 Oct 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants