Skip to content

Add CumSum and Round for Opset 11 - #1705

Merged
jignparm merged 15 commits into
masterfrom
jignparm/cumsum_round_official
Sep 26, 2019
Merged

Add CumSum and Round for Opset 11#1705
jignparm merged 15 commits into
masterfrom
jignparm/cumsum_round_official

Conversation

@jignparm

Copy link
Copy Markdown
Contributor

Description: CumSum() and Round() are new operators for ONNX opset 11

Motivation and Context
Add implementations of new operators to OnnxRuntime.

@jignparm
jignparm requested a review from a team as a code owner August 28, 2019 01:33
@fs-eire

fs-eire commented Aug 28, 2019

Copy link
Copy Markdown
Contributor

Are this PR a duplicated with #1539 and #1536?

Comment thread onnxruntime/test/python/onnx_backend_test_series.py
Comment thread onnxruntime/test/onnx/main.cc Outdated
Comment thread onnxruntime/core/providers/cpu/cpu_execution_provider.cc
Comment thread onnxruntime/core/providers/cpu/math/cumsum.cc Outdated
Comment thread onnxruntime/core/providers/cpu/math/cumsum.h Outdated
Comment thread onnxruntime/core/providers/cpu/math/round.cc Outdated
Comment thread onnxruntime/core/providers/cpu/tensor/utils.h Outdated
Status Round<MLFloat16>::Compute(OpKernelContext* ctx) const {
const auto& X = *ctx->Input<Tensor>(0);
auto& Y = *ctx->Output(0, X.Shape());
auto input = X.template Data<MLFloat16>();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are several places (other than this line) where RHS is of type ptr and LHS is just auto. As per the coding guidelines we should "Qualify usages of 'auto' with 'const', '*', '&' and '&&' where applicable to more clearly express the intent".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in the other locations as well, to make it more clear which auto vars are consts and pointers.

WritableSliceIterator(Tensor& tensor, const TensorShape& tensor_shape, gsl::span<const int64_t> starts,
gsl::span<const int64_t> extents, gsl::span<const int64_t> steps)
: tensor_(tensor), input_(tensor_.template MutableData<T>()), extents_(extents), skips_(tensor_shape, extents, steps), indices_(extents.size(), 0) {
auto& dims = tensor_shape.GetDims();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const auto&

WritableSliceIterator(Tensor& tensor, gsl::span<const int64_t> starts,
gsl::span<const int64_t> extents, gsl::span<const int64_t> steps)
: tensor_(tensor), input_(tensor_.template MutableData<T>()), extents_(extents), skips_(tensor_.Shape(), extents, steps), indices_(extents.size(), 0) {
auto& dims = tensor_.Shape().GetDims();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const auto&

@jignparm
jignparm merged commit 80ef629 into master Sep 26, 2019
@jignparm
jignparm deleted the jignparm/cumsum_round_official branch September 26, 2019 09:46
yuslepukhin pushed a commit that referenced this pull request Mar 17, 2026
## Describe your changes
Bugfix in quantized model export. Bias weight was not set correctly
before.

## 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
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

Successfully merging this pull request may close these issues.

5 participants