-
Notifications
You must be signed in to change notification settings - Fork 22
Initial implementation of Deformable Convolution workload #168
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
Initial implementation of Deformable Convolution workload #168
Conversation
| "deformable_convolution_sycl", | ||
| ] | ||
|
|
||
| """l2-norm calculation of n vectors |
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.
Comments need to be added for this workload
|
|
||
| return ( | ||
| default_rng.random((batch, in_channels, in_height, in_width)).astype(dtype), | ||
| # np.ones((batch, in_channels, in_height, in_width)).astype(dtype), |
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.
Please cleanup commented out code.
| @@ -0,0 +1,81 @@ | |||
| # Copyright 2022 Intel Corp. | |||
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.
Looks like this _numpy implementation is the same as the _numba_npr version. If so, we should not be adding this.
| ) | ||
| find_package(MKL CONFIG REQUIRED) | ||
|
|
||
| # target_include_directories(${py_module_name} PUBLIC ${Dpctl_INCLUDE_DIRS} $) |
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.
Please remove commented out lines
| @@ -0,0 +1,297 @@ | |||
| //==- _l2_norm_sycl.cpp - Python native extension of l2-norm ===// | |||
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.
Update comments to reflect this workload
| // SPDX - License - Identifier : Apache 2.0 | ||
| /// | ||
| /// \file | ||
| /// The files implements a SYCL-based Python native extension for the |
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.
Update comments
|
|
||
| auto offset_y = *get_ptr_5d(offset, k_height, k_width, 2, out_height, out_width, kh, kw, 1, h, w) + h*stride_y + (kh - k_h_m)*dilation_y - (pad_y - k_h_m); | ||
| auto offset_x = *get_ptr_5d(offset, k_height, k_width, 2, out_height, out_width, kh, kw, 0, h, w) + w*stride_x + (kw - k_w_m)*dilation_x - (pad_x - k_w_m); | ||
| // auto offset_y = h*stride_y + (kh - k_h_m)*dilation_y - (pad_y - k_h_m); |
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.
Remove commented lines
| auto _input = get_ptr_3d(input, in_channels, in_height, in_width, c, 0, 0); | ||
|
|
||
| _output[w] = bilinear(_input, in_height, in_width, offset_y, offset_x); | ||
| // _output[w] = offset_y; |
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.
Remove line
dpbench/runner.py
Outdated
| + " ========================" | ||
| ) | ||
| if result.error_state == 0: | ||
| if result.error_state == enums.ErrorCodes.SUCCESS: |
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 change does not appear to be part of this workload PR. Consider adding separate PR.
5ac57f4 to
031e9f8
Compare
031e9f8 to
5c6c720
Compare
|
@AlexanderKalistratov thank you for you PR. Could you please update implementation (@diptorupd said that you've improved it). Please also get rid of unrelated comments, update comments to match current implementation and provide PR description. Hoping to see updates in the nearest feature! |
3c0758e to
d525b2c
Compare
bee3199 to
01ce90e
Compare
cc17ce1 to
424694e
Compare
424694e to
c5bddb3
Compare
|
@ZzEeKkAa deformable convolution can't pass CI because it uses |
|
M size is too small compared to other WLs, I suggest to move L to M and make L even bigger |
|
Thank @AlexanderKalistratov and @Hardcode84. Is it hard to add numpy implementation (using numpy features)? |
|
Are MKL AND TBB cmake files patched? If no and compilation is failing we need to update workflow, not provide copy of cmake files. I guess dpnp or dpctl does it in conda recepie, so we can refer there. |
Numpy version will be prohibitively slow even for S size. |
As Ivan said straightforward implementation would always fail with timeout even on S preset. I had one and then remove it. I'd say we need option to exclude workload from run. Right now we have option to list workloads to run and now we need also option to list workloads NOT to run. And then simply exclude deformable convolution from no-sycl runs.
They are patched. These cmakes taken from dpnp repo. |
|
Also we need to handle the case when reference implementation not found more gracefully than now |
|
Closing this PR. Please reopen if there are further updates. |
No description provided.