-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[TOPI] LRN & L2norm Operator #1000
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
Conversation
|
@sxjscience can you help review this? |
|
I'll check the formula next week... This week I decide to focus on the MT demo. |
|
Looks that the functionality can be implemented by stacking multiple reduce/broadcast ops in TOPI. |
|
@sxjscience Using broadcast/reduce I feel it will be difficult since we need to compute for a selected area based on the "local size" parameter, thats why i used reduce_axis to do the sum operation. Can you give a reference how to use broadcast/reduce operator in this scenario? |
|
Yes, you are correct. Stacking multiple reduce/bcast seems to be infeasible. I’ll check the implementation in detail this week. Currently too busy with other works...
Get Outlook for iOS<https://aka.ms/o0ukef>
…________________________________
From: Pariksheet Pinjari <notifications@github.com>
Sent: Sunday, March 18, 2018 11:32:47 PM
To: dmlc/tvm
Cc: Xingjian SHI; Mention
Subject: Re: [dmlc/tvm] [TOPI] LRN Operator (#1000)
@sxjscience<https://github.com/sxjscience> Using broadcast/reduce I feel it will be difficult since we need to compute for a selected area based on the "local size" parameter, thats why i used reduce_axis to do the sum operation. Can you give a reference how to use broadcast/reduce operator in this scenario?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#1000 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AE8D7nEnQ6OrqVZcDjMgF707rdSGCTXCks5tf1EPgaJpZM4SpTLK>.
|
|
I will upload L2norm operator implementation also along with this pull request, please help to review together. |
|
ping @sxjscience can you take another look |
topi/python/topi/nn/l2_norm.py
Outdated
| import tvm | ||
|
|
||
| @tvm.target.generic_func | ||
| def l2norm_instance_nchw(data, eps): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should implement L2Norm with arbitrary shape and axis.
|
|
||
| ##Add padding on left & right of size radius first | ||
| pad_before = [0, int(size/2), 0, 0] | ||
| pad_after = [0, int(size/2), 0, 0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use size // 2 instead of int(size/2)
| rxk = tvm.reduce_axis((0, size), name='rxk') | ||
| sqr_sum = tvm.compute((b, c, h, w), lambda i, l, j, k: tvm.sum( | ||
| pad_data[i, l + rxk, j, k] * pad_data[i, l + rxk, j, k], | ||
| axis=rxk)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can first use square and then use topi.sum, for example: I've misunderstood the code. We cannot do this.
| lrn_out[i, c, j, k] = a_np[i, c, j, k] / \ | ||
| sqr_sum_up[i, c, j, k] | ||
|
|
||
| return lrn_out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should move the python implementation to test_topi_lrn.py.
topi/tests/python/test_topi_lrn.py
Outdated
| b = tvm.nd.array(np.zeros(get_const_tuple(B.shape), dtype=dtype), ctx) | ||
| f = tvm.build(s, [A, B], device) | ||
| f(a, b) | ||
| np.testing.assert_allclose(b.asnumpy(), b_np, rtol=1e-1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel that rtol=1e-1 is too loose. We should set it to a smaller value like 1E-4 or 1E-3.
topi/python/topi/nn/l2_norm.py
Outdated
|
|
||
| @tvm.target.generic_func | ||
| def l2norm_instance_nchw(data, eps): | ||
| """Perform local response normalisation on the data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is L2Norm instead of local response normalization.
|
|
||
| @tvm.target.generic_func | ||
| def lrn_nchw(data, size, alpha=0.0001, beta=0.75, bias=2): | ||
| """Perform local response normalisation on the data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should write down the formula like in PyTorch (https://github.com/pytorch/pytorch/blob/master/torch/nn/modules/normalization.py#L9-L36) and TensorFlow (https://www.tensorflow.org/api_docs/python/tf/nn/local_response_normalization).
| (bias + (alpha * sqr_sum[i, j, k, l] / size)), beta)) | ||
|
|
||
| return tvm.compute(data.shape, | ||
| lambda b, c, h, w: data[b, c, h, w] / sqr_sum_up[b, c, h, w]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can reuse the implemented binary broadcasting operators like topi.broadcast_div.
40e2ac7 to
1d6df5a
Compare
|
@tqchen can you please reopen this PR, i accidentally closed it. |
|
Review comments are updated in #1051 |
LRN operator for caffe alexnet model to use with onnx frontend