MIOpenGEMM icon indicating copy to clipboard operation
MIOpenGEMM copied to clipboard

The distance calculation in the get_distance is probably incorrect

Open zjing14 opened this issue 6 years ago • 1 comments

I found there may be bugs (see below) in the get_distance function of MIOpenGEMM that makes MIOpenGEMM cannot find the best config in the kernel cache. I think there are some “==” should be “!=”.

double Geometry::get_distance(const Geometry& g2) const
{
 
  double distance = 0;
  if (same_transposes(g2) == false)
  {
    distance = std::numeric_limits<double>::max();
  }
 
  else
  {
 
    for (unsigned i = 0; i < 6; ++i)
    {
      distance += std::abs(metric_co[i] - g2.metric_co[i]);
    }
    for (size_t x : {2, 4, 8})
    {
      for (auto emat : {Mat::E::A, Mat::E::B, Mat::E::C})
      {
        distance += 0.2 * ((ldX[emat] % x == 0) != (g2.ldX[emat] % x == 0));
      }
    }
 
    for (size_t x : {256, 512, 1024})
    {
      for (auto emat : {Mat::E::A, Mat::E::B, Mat::E::C})
      {
       //should be “!=” here
        distance += 0.2 * (std::min<size_t>(ldX[emat] % x, x - ldX[emat] % x) % 4 ==
                           std::min<size_t>(g2.ldX[emat] % x, x - g2.ldX[emat] % x) % 4);
      }
    }
 
    for (size_t i = 0; i < wSpaceSufficient.size(); ++i)
{
     //should be “!=” here
      distance += 0.2 * (wSpaceSufficient[i] == g2.wSpaceSufficient[i]);
    }
  }
 
  distance += 1e-5 * (std::log(wSpaceSize + 1.1) - std::log(g2.wSpaceSize + 1.1));
 
  return distance;
}

zjing14 avatar Mar 12 '18 21:03 zjing14

I think you are correct. Please make the changes you suggest and open a Pull Request.

I also wonder if the last term before return is sensible, perhaps absolute value of the r.h.s. would be better.

Thanks, James

newling avatar Mar 12 '18 21:03 newling