plaidml / openvino

OpenVINO™ Toolkit - Deep Learning Deployment Toolkit repository
https://docs.openvinotoolkit.org/latest/index.html
Apache License 2.0
0 stars 1 forks source link

add EmbeddingBagOffsetsSum op and tests #100

Closed haoyouab closed 3 years ago

haoyouab commented 4 years ago

Some test cases need more discussion and clarification. Please see inline comments.

haoyouab commented 3 years ago

Should this be reverted or should I just open a new PR for revision?

By the way, I wonder how I can use op::slice in the way shown below:

auto I_gathered = gather(I, indices);
std::vector<edsl::Tensor> Os;

for (uint32_t i = 0; i < batch; ++i) {
  Tensor slice = op::slice(I_gathered).add_dim(offsets[i], offsets[i + 1])
        .add_dim()
        .add_dim()
        ...;    ----> The rank of I_gathered is variadic though.
  auto reduced = op::sum(slice, edsl::make_tuple(0));
  Os.push_back(reduced);
}

auto O = op::concatenate(Os, 0);
return edsl::make_tuple(O);

Doesn't op::slice support slicing just on a specific dimension using just one add_dim and leaving the rest as is (i.e. seems we must provide as many add_dims as the number of rank)? Now I use edsl::Contraction but it looks not that concise. Please check this commit. Thanks!

tzerrell commented 3 years ago

I don't think a reversion is necessary; opening a new PR with revisions should be sufficient.

I'll get back to you on your other comments once I have a chance to look more carefully.