Generalize CategoryMapper test.

[etiotto]

In this PR I generalized the test infrastructure for the CategoryMapper operator by providing templated functions to cut down on code duplication. As a side effect I modified the runtime interfaces for querying OMTensor properties to pass input OMTensor(s) with const.

Signed-off-by: Ettore Tiotto [email protected]

[etiotto]

@NathanielMcVicar @sstamenova I saw this error on the Win CI:

##[error]No hosted parallelism has been purchased or granted. To request a free parallelism grant, please fill out the following form https://aka.ms/azpipelines-parallelism-request

Maybe is transitory?

[NathanielMcVicar]

Maybe is transitory?

Unfortunately, it doesn't seem to be. I'm going to fill out the request form since this seems to meet the definition of a public project. I suspect when we moved over to an ONNX build subscription to host more artifacts we lost our 2 free parallel jobs.

[NathanielMcVicar]

This appears to be a wide issue effecting AzDO pipelines across organizations. The ONNX team is aware and working to rootcause the issue. See for example conda-forge/conda-forge.github.io#1604. Thanks @jcwchen!

[etiotto]

This appears to be a wide issue effecting AzDO pipelines across organizations. The ONNX team is aware and working to rootcause the issue. See for example conda-forge/conda-forge.github.io#1604. Thanks @jcwchen!

Thank you very much @NathanielMcVicar and @jcwchen for the quick response and investigation.

[NathanielMcVicar]

@etiotto Windows-CI issues are resolved.

[AlexandreEichenberger]

Can you comment on if that cast is safer/ok compared to what was done in numerical, namely

  for (int64_t i = 0; i < I; ++i) {
    for (int64_t j = 0; j < J; ++j) {
      omTensorGetElem<float>(ref, {i, j}) = 0;
      for (int64_t k = 0; k < K; k++) {
        float aVal, bVal;
        if (aTrans == 0)
          aVal = omTensorGetElem<float>(a.get(), {i, k});
        else
          aVal = omTensorGetElem<float>(a.get(), {k, i});
        if (bTrans == 0)
          bVal = omTensorGetElem<float>(b.get(), {k, j});
        else
          bVal = omTensorGetElem<float>(b.get(), {j, k});
        omTensorGetElem<float>(ref, {i, j}) += aVal * bVal;
      }
    }
  }

I agree that the quoted code above is dreadfully slow by creating a vector for each accesses

[etiotto]

The implementation of omTensorGetElem is:

template <typename T>
T &omTensorGetElem(const OMTensor *omt, std::vector<int64_t> indexes) {
  int64_t elemOffset = omTensorComputeElemOffset(omt, indexes);
  return ((T *)omt->_alignedPtr)[elemOffset];
}

The type cast on the return stmt is in C style. A C-style cast (T *) is not equivalent to a C++ static_cast<T*>() because a C-style cast is not type safe, while a static_cast allows the compiler to report errors. For example:

void foo(char *p) {
  int *pi = (int *)p;   // no compiler error here
  *pi = 3;              // ooops: just overwrote 3 extra bytes in mem. pointed to by p!!!
}

If a static_cast<int*>(p) was used the compiler would catch the potential runtime problem at compile time ==> safer!

If the intention was to "reinterpret" the memory pointed to by p then one should use a C++ reintepret_cast.

Of course all of this implies the code section is compiled by a C++ compiler.

[etiotto]

We prob. should modify the runtime implementation of omTensorGetElem and use C++ style casts. But that is another PR.

[AlexandreEichenberger]

We prob. should modify the runtime implementation of omTensorGetElem and use C++ style casts. But that is another PR.

That file is unique, it has to be compiled in C or C++, with some of the code which is an "extension" and is only for C++ with header defined in a separate helper header file...

[etiotto]

ok

[AlexandreEichenberger]

LGTM

[etiotto]

@jenkins-droid test this please

[jenkins-droid]

Jenkins Linux s390x Build #3854 [push] Generalize CategoryMappe... started at 09:14

[jenkins-droid]

Jenkins Linux amd64 Build #3863 [push] Generalize CategoryMappe... started at 08:14

[jenkins-droid]

Jenkins Linux ppc64le Build #2970 [push] Generalize CategoryMappe... started at 09:15

[jenkins-droid]

Jenkins Linux s390x Build #3854 [push] Generalize CategoryMappe... passed after 37 min

[jenkins-droid]

Jenkins Linux ppc64le Build #2970 [push] Generalize CategoryMappe... passed after 47 min

[jenkins-droid]

Jenkins Linux amd64 Build #3863 [push] Generalize CategoryMappe... passed after 52 min