Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Diff Test, expected collisions do not affect the testing result #199

Open
gzm55 opened this issue May 17, 2021 · 2 comments
Open

Diff Test, expected collisions do not affect the testing result #199

gzm55 opened this issue May 17, 2021 · 2 comments
Assignees
@rurban

Comments

@gzm55
Copy link

gzm55 commented May 17, 2021

In Diff Test, we have an excepted collision count:

smhasher/DifferentialTest.h

Line 141 in d428f2f

double expected = testcount / pow(2.0,double(hashbits));

But this variable seems do not affect the testing result:

smhasher/DifferentialTest.h

Line 44 in d428f2f

if(count > 1)

and the last group collisions are also skipped for unsetting the result.

smhasher/DifferentialTest.h

Line 68 in d428f2f

double pct = 100 * (double(count) / double(reps));

@rurban rurban self-assigned this May 17, 2021
@rurban
Copy link
Owner

rurban commented May 23, 2021

Yes, the calculation of expected in line 141 is only for the print before we actually check the collisions.

We don't check against the float calc of expected, we check only against 3 or more collisions as described in the comment.
Sort through the differentials, ignoring collisions that only occured once (these could be false positives). If we find collisions of 3 or more, the differential test fails.

A much simpler test.

@gzm55
Copy link
Author

gzm55 commented May 23, 2021

ok, but the last group should be counted, in the block including line 68.

fwojcik added a commit to fwojcik/smhasher3 that referenced this issue Aug 16, 2022
@fwojcik
CRITICAL FIX: Not all differentials can cause Diff test failures
38738c4 
This is reported bug #199:  rurban/smhasher#199

To see this bug in action, apply the following patch (without this fix!):
@@ -158,7 +158,7 @@ bool DiffTest ( pfHash hash, int diffbits, int reps, bool dumpCollisions )
          reps,testcount,expected);

   Hash_Seed_init (hash, g_seed);
-  for(int i = 0; i < reps; i++)
+  for(int i = 0; i < 0; i++)
   {
     if(i % (reps/10) == 0) printf(".");

@@ -173,6 +173,10 @@ bool DiffTest ( pfHash hash, int diffbits, int reps, bool dumpCollisions )

   bool result = true;

+  r.rand_p(&k1,sizeof(keytype));
+  diffs.push_back(k1);
+  diffs.push_back(k1);
+
   result &= ProcessDifferentials(diffs,reps,dumpCollisions);

   return result;
rurban pushed a commit that referenced this issue Aug 17, 2022
@fwojcik @rurban
CRITICAL FIX: Not all differentials can cause Diff test failures
750fe8f 
This is reported bug #199:  #199

To see this bug in action, apply the following patch (without this fix!):
@@ -158,7 +158,7 @@ bool DiffTest ( pfHash hash, int diffbits, int reps, bool dumpCollisions )
          reps,testcount,expected);

   Hash_Seed_init (hash, g_seed);
-  for(int i = 0; i < reps; i++)
+  for(int i = 0; i < 0; i++)
   {
     if(i % (reps/10) == 0) printf(".");

@@ -173,6 +173,10 @@ bool DiffTest ( pfHash hash, int diffbits, int reps, bool dumpCollisions )

   bool result = true;

+  r.rand_p(&k1,sizeof(keytype));
+  diffs.push_back(k1);
+  diffs.push_back(k1);
+
   result &= ProcessDifferentials(diffs,reps,dumpCollisions);

   return result;
fwojcik added a commit to fwojcik/smhasher that referenced this issue Aug 18, 2022
@fwojcik
CRITICAL FIX: Not all differentials can cause Diff test failures
e3036ec 
This is reported bug rurban#199:  rurban#199

To see this bug in action, apply the following patch (without this fix!):
@@ -158,7 +158,7 @@ bool DiffTest ( pfHash hash, int diffbits, int reps, bool dumpCollisions )
          reps,testcount,expected);

   Hash_Seed_init (hash, g_seed);
-  for(int i = 0; i < reps; i++)
+  for(int i = 0; i < 0; i++)
   {
     if(i % (reps/10) == 0) printf(".");

@@ -173,6 +173,10 @@ bool DiffTest ( pfHash hash, int diffbits, int reps, bool dumpCollisions )

   bool result = true;

+  r.rand_p(&k1,sizeof(keytype));
+  diffs.push_back(k1);
+  diffs.push_back(k1);
+
   result &= ProcessDifferentials(diffs,reps,dumpCollisions);

   return result;
@fwojcik fwojcik mentioned this issue Aug 18, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rurban rurban

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@rurban @gzm55