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.

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

[onert] Incorrect use of Pad data in cpu backend #12541

Closed
ragmani opened this issue Jan 29, 2024 · 3 comments
Closed

[onert] Incorrect use of Pad data in cpu backend #12541

ragmani opened this issue Jan 29, 2024 · 3 comments
Assignees

Comments

@ragmani
Copy link
Contributor

ragmani commented Jan 29, 2024

What

As we discussed offline, In circle schema, pad data of Pad operation is a tensor, not PadOption. So pad data must be used as a tensor instead of constant data in cpu backend.

Originally posted by @ragmani in #12535 (comment)

/cc @YongseopKim

@ragmani ragmani changed the title [onert] Wrong Pad data in cpu backend [onert] Incorrect use of Pad data in cpu backend Jan 29, 2024
@YongseopKim YongseopKim self-assigned this Jan 29, 2024
@YongseopKim
Copy link
Contributor

While #12413, I'll fix it.

@YongseopKim
Copy link
Contributor

I've instigated so far, I figure out that pad op's pad data is used as const in many tests.

An example.

// Pad.test.cc
TEST_P(PadVariation, Test)
{
  auto &param = GetParam();

  CircleGen cgen;
  int in = cgen.addTensor({{1, 2, 2, 1}, param.data_type}, param.scale, param.zero_point);
  std::vector<int32_t> padding_data{0, 0, 1, 1, 1, 1, 0, 0};
  uint32_t padding_buf = cgen.addBuffer(padding_data);
  int padding = cgen.addTensor({{4, 2}, circle::TensorType::TensorType_INT32, padding_buf}); // **HERE**
  int out = cgen.addTensor({{1, 4, 4, 1}, param.data_type}, param.scale, param.zero_point);

  cgen.addOperatorPad({{in, padding}, {out}});
  cgen.setInputsAndOutputs({in}, {out});
  _context = std::make_unique<GenModelTestContext>(cgen.finish());
  _context->addTestCase(param.tcd);
  _context->setBackends({"acl_cl", "acl_neon", "cpu"});

  SUCCEED();
}
// Other tests also handle pad data as const data

The best way I think now is to support both tensor and const.

Feel free to write comments if I misunderstand.

@YongseopKim
Copy link
Contributor

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants