[math] look at node in proper order

[ferdymercury]

This Pull request:

Changes or fixes:

This solution copies the same structure from lines 590 where the if-else statements are doing something different.

See

root/math/mathcore/src/TKDTree.cxx

Lines 575 to 581 in 65b3913

	if (point[fAxis[inode]]<fValue[inode]){
	//first examine the node that contains the point
	UpdateNearestNeighbors(GetLeft(inode), point, kNN, ind, dist);
	UpdateNearestNeighbors(GetRight(inode), point, kNN, ind, dist);
	} else {
	UpdateNearestNeighbors(GetRight(inode), point, kNN, ind, dist);
	UpdateNearestNeighbors(GetLeft(inode), point, kNN, ind, dist);

Fixes https://its.cern.ch/jira/browse/ROOT-10374

Checklist:

• tested changes locally
• updated the docs (if necessary)

[github-actions]

Test Results

    21 files      21 suites   3d 6h 34m 47s ⏱️
 3 223 tests  3 223 ✅ 0 💤 0 ❌
65 957 runs  65 957 ✅ 0 💤 0 ❌

Results for commit 573a314.

♻️ This comment has been updated with latest results.

[lmoneta]

It is probably correct, but can we have a simple test showing the problem existing before?
Also, why the condition is now from <= to < ?

[ferdymercury]

Thanks for the review

Also, why the condition is now from <= to < ?

To be consistent with the block starting at line 590.

It is probably correct, but can we have a simple test showing the problem existing before?

I think I do not know enough of this class to prepare a test, sorry :s

[ferdymercury]

@lmoneta:
The following is the effect. Just mkdir /tmp/before /tmp/after, then compile without this PR patch, run the test below, then compile with this patch, switch the foldername in the test below, and rerun the test. Then you can do "meld" /tmp/before //tmp/after. You will see how the order in which "res" is filled changes.

So, the sorting is more "optimized" after this patch, even if the total number of elements does not change.

void TestRange()
{
//Test TKDTree::FindInRange() function

   Int_t npoints = 1000;
   Double_t range = 20;

   printf("%d points, range=%f\n", npoints, range);
   Int_t ntimes = 5000;
   Double_t *x = new Double_t[npoints];
   Double_t *y = new Double_t[npoints];
   Double_t *z = new Double_t[npoints];
   for (Int_t i=0; i<npoints; i++){
      x[i] = 15 + i;
      y[i] = 20 + i;
      z[i] = 25 + i;
   }

   Int_t *results1 = new Int_t[npoints];
   std::vector<Int_t> results2;
   Int_t np1;

//Compute with the kd-tree
   Int_t bsize = 10;
   TKDTreeID *kdtree = new TKDTreeID(npoints, 3, bsize);
   kdtree->SetData(0, x);
   kdtree->SetData(1, y);
   kdtree->SetData(2, z);
   kdtree->Build();
   Double_t *dist = new Double_t[npoints];
   Int_t *index = new Int_t[npoints];
   Int_t ndiff = 0;
   for (Int_t itime=0; itime<ntimes; itime++){
      Double_t point[3];
      point[0]=15 + itime;
      point[1]=20 + itime;
      point[2]=25 + itime;

      //printf("point: (%f, %f, %f)\n\n", point[0], point[1], point[2]);
      for (Int_t ipoint=0; ipoint<npoints; ipoint++){
         dist[ipoint]=0;
         dist[ipoint]+=(x[ipoint]-point[0])*(x[ipoint]-point[0]);
         dist[ipoint]+=(y[ipoint]-point[1])*(y[ipoint]-point[1]);
         dist[ipoint]+=(z[ipoint]-point[2])*(z[ipoint]-point[2]);
         dist[ipoint]=TMath::Sqrt(dist[ipoint]);
         index[ipoint]=ipoint;
      }
      TMath::Sort(npoints, dist, index, kFALSE);
      np1=0;
      while (np1<npoints && dist[index[np1]]<=range){
         results1[np1]=index[np1];
         np1++;
      }
      results2.clear();
      kdtree->FindInRange(point, range, results2);
      std::ofstream gout("/tmp/before/"+std::to_string(itime)+".txt");
     // std::ofstream gout("/tmp/after/"+std::to_string(itime)+".txt");
      for(auto re : results2) gout << re << std::endl;
      gout.close();

      if (TMath::Abs(np1 - Int_t(results2.size()))>0.1) {
         ndiff++;
         printf("different numbers of points found, %d %d\n", np1, Int_t(results2.size()));
         continue;
      }

      //have to sort the results, as they are in different order
      TMath::Sort(np1, results1, index, kFALSE);
      std::sort(results2.begin(), results2.end());

      for (Int_t i=0; i<np1; i++){
         if (TMath::Abs(results1[index[i]]-results2[i])>1E-8) ndiff++;
      }
   }
   printf("%d  differences found between \"brute force\" method and kd-tree\n", ndiff);

   delete [] x;
   delete [] y;
   delete [] z;
   delete [] index;
   delete [] dist;
   delete [] results1;
   delete kdtree;
}