MV-473: [MV-MIOpen] Benchmark & Port Interpolate #41
Conversation
|
Wow the improvement is very big. Could you provide script you used to benchmark rocm vs modnn + script benchmark rocm vs miopen to here or comment on your Jira ticket (recommend this method because recently our workflow required uploading benchmark script to comment of Jira ticket). Did you find any specific reasons that the improvement in some cases is very big? For example I saw linear backward is around 200x improvement compared to ROCm |
Very nice of you to remind me. I added the script to JIRA ticket. There are no special reasons; my MIOpen's performance is similar to Modnn's performance, and I made some constraints to eliminate low-performance cases. |
|
Wow the numbers look very impressive. But Some of that don't make sense to me, for example 99.29x in fp16 and 2.59x in fp32 (Bilinear bwd). Can you figure out why this much difference happen? It looks like one case with 1800x improvement ruined the geomean but please check. |
|
I read benchmark number in 5d backward nearest and it looks like the last (very) small tests contributed too much to your average improvement. Overall big tests produced slower results (around 0.8 - 1x improvement) but the last very small tests with only 2048 and 4096 threads have 10-14x improvement, and then average improvement for this case is 2.66x, which I think is not fair. Could you create more tests for that cases with total threads between 100k and 5M to see if it is faster or not? |
|
Run |
|
Please don't update this branch to newest commit in upstream repo. Because in this PR you choose to merge to develop-moreh branch, now it will show '752 file changed' and I cannot review your code =))) (I clicked Files changed button and my Edge crashed after that 😡) You can consider creating a new PR which merge to moreh-dev:develop if you want |
If I don't merge to develop, I can't fix issues which you mentioned above about MIOPEN_INTERNALS_EXPORT or Gtest new convention. You reap what you sow, my dear. Love your thoughtful reviews btw <3 ! |
Ok bro 🥲 if that is you can ignore my comments which can only fix when updating to new branch, i thought that macro existed from older branch, my bad 🥹 please revert back to previous commit before you updated to latest commit on upstream repo because now I have no ways to review your PR 😂😂 And i rethink it’s not a good idea to open a new PR to moreh-dev:develop because that branch is not synced up to latest commit on upstream repo, if your last commit you update to branch develop on moreh-dev then it is ok to open a new PR, but if you update to branch develop on rocm then it will show up many new files not related to your operation when opening new PR to moreh-dev:develop |
hieule88
force-pushed
the
impl_interpolate_final
branch
from
600728c
to
4d008c8
Compare
|
Overall it looks good now. Make sure to check this comment #41 (comment) and update 5D backward nearest benchmark result after you create more test cases. Also I found that in the newest commit d15fbc5 you commented out |
I commented for re-run benchmark, updated 5D backward, check it out |
|
LGTM! Thank you for your hard work in this PR, and make sure to ask for review from other reviewers. |
|
@kyeonghwanryu @DuongQLee Do you want to add more reviews? If not, can I close this PR? |
|
Sorry it seems I missed the comment. I'm closing it. |
Added Interpolate forward and backward with multi-mode operation and kernel.
Added driver test and gtest for Interpolate .
New API is guarded by MIOPEN_BETA_API macro.
Compared to ROCm pytorch: Link
Average over all cases:
Mode Nearest