Conversation
This change added GatherElements (op_ver 11) to the Nuphar provider.
| namespace tvm_codegen { | ||
|
|
||
| tvm::Tensor GatherElements(const tvm::Tensor& t, | ||
| int64_t axis, |
There was a problem hiding this comment.
axis [](start = 35, length = 4)
can this be negative? if not, maybe should use unsigned types?
There was a problem hiding this comment.
We always use int64_t for axis in other tensor apis. Seems it's better to follow the convention.
| {"a", "a", | ||
| "d", "d"}); | ||
| test4.Run(OpTester::ExpectResult::kExpectFailure, "GatherElements op: Value in indices must be within bounds [-2 , 1]. Actual value is -3"); | ||
| // skip nuphar, which will not throw error message but will ensure no out-of-bound access |
There was a problem hiding this comment.
which will not throw error message but will ensure no out-of-bound access [](start = 18, length = 73)
I believe this should be the default behavior in ONNX spec for this op. Lots of devices won't be able to throw on invalid indices. Please open an issue to ONNX.
| tvm::Expr real_idx = tvm::max(tvm::min(idx, idx_upper_bound - 1), 0) + | ||
| (idx < 0) * (idx_upper_bound + tvm::max(idx, -idx_upper_bound)); | ||
| // tvm idx must be of Int(32) | ||
| ivars.push_back(tvm::cast(tvm::Int(32), real_idx)); |
There was a problem hiding this comment.
maybe better to make it a common util for index clamping, as other ops may reuse it too (Gather?) #Resolved
There was a problem hiding this comment.
Done. #Resolved
* create a utilify function for accessing index safely
| } | ||
|
|
||
| // Make sure idx is within the range of [-bound, bound - 1] | ||
| tvm::Expr SafeIndex(const tvm::Expr& idx, const tvm::Expr& bound); |
There was a problem hiding this comment.
SafeIndex [](start = 10, length = 9)
Clamp index to be more exact
| return fixed_inputs; | ||
| } | ||
|
|
||
| // Make sure idx is within the range of [-bound, bound - 1] |
There was a problem hiding this comment.
within [](start = 20, length = 6)
clamped in
* SafeIndex -> ClampIndex
## Describe your changes Adding OV recipes for Phi4, Phi4-mini-reasoning, several variants of qwen2.5 instruct & qwen2.5 coder instruct, deepseek, and mistral. ## Checklist before requesting a review - [ ] Add unit tests for this change. - [ ] Make sure all tests can pass. - [ ] Update documents if necessary. - [ ] Lint and apply fixes to your code by running `lintrunner -a` - [ ] Is this a user-facing change? If yes, give a description of this change to be included in the release notes. - [ ] Is this PR including examples changes? If yes, please remember to update [example documentation](https://github.com/microsoft/Olive/blob/main/docs/source/examples.md) in a follow-up PR. ## (Optional) Issue link --------- Co-authored-by: Anirudh Swaminathan <anirudh.swaminathan@intel.com> Co-authored-by: Kaunain <kaunain.ahmed@intel.com> Co-authored-by: kaunain-98 <108542306+kaunain-98@users.noreply.github.com>
2 participants
This change added GatherElements (op_ver 11) to the Nuphar provider.