Skip to content

Conversation

@PariksheetPinjari909
Copy link
Contributor

LRN operator for caffe alexnet model to use with onnx frontend

@tqchen @sxjscience your review comments from #1000 is addressed here. #1000 got accidentally closed.

@tqchen
Copy link
Member

tqchen commented Mar 27, 2018

cc @sxjscience can you do another round of review?

@sxjscience
Copy link
Member

sxjscience commented Mar 27, 2018

Need to work on the project in Amazon. I'll give a deep dive into how to implement it better on Thursday. For now, I think we need to support L2Normalization with arbitrary axis.

@PariksheetPinjari909
Copy link
Contributor Author

@sxjscience , shall I follow https://www.tensorflow.org/api_docs/python/tf/nn/l2_normalize for l2norm implementation?

@PariksheetPinjari909
Copy link
Contributor Author

@sxjscience , I have added arbitrary axis support for L2norm. Could you please help to review further.

@sxjscience
Copy link
Member

I think it's good now. We can refactor the implementation later.

Copy link
Member

@tqchen tqchen left a comment

Choose a reason for hiding this comment

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

It seems that we can also make lrn axis invariant by providing an axis argument for to specify the channel, can we do that?


rxk = tvm.reduce_axis((0, size), name='rxk')
sqr_sum = tvm.compute((b, c, h, w), lambda i, l, j, k: tvm.sum(
tvm.power(pad_data[i, l + rxk, j, k], 2.0),
Copy link
Member

Choose a reason for hiding this comment

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

do not use power to calculate the square, instead use x* x

sqr_sum[i, j, k, l] = sqr_sum[i, j, k, l] + \
(a_np[i, j, k, l + rxl] * \
a_np[i, j, k, l + rxl])
for i in range(axis0):
Copy link
Member

Choose a reason for hiding this comment

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

use broadcasting semantics, instead of the loop

@tqchen tqchen merged commit b242b94 into apache:master Apr 13, 2018
@tqchen
Copy link
Member

tqchen commented Apr 13, 2018

Thanks for improving the code during the review process, this is now merged!

@tqchen tqchen mentioned this pull request May 29, 2018
tqchen pushed a commit to tqchen/tvm that referenced this pull request Jul 6, 2018
sergei-mironov pushed a commit to sergei-mironov/tvm that referenced this pull request Aug 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants