Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

[Opperf] Add array rearrange operators to opperf #15606

Merged
merged 5 commits into from
Jul 25, 2019

Conversation

ChaiBapchya
Copy link
Contributor

@ChaiBapchya ChaiBapchya commented Jul 19, 2019

Description

Added Rearray operators to the existing set of opperf benchmarks

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • Changes are complete (i.e. I finished coding on this PR)
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • Added following ops
  1. transpose
  2. swapaxes
  3. flip
  4. depth_to_space
  5. space_to_depth

Results

GPU - https://gist.github.com/ChaiBapchya/51b6c7c2346959f3f72aa1b702eb002d
CPU - https://gist.github.com/ChaiBapchya/37b27d4803e73e0fa5a8bd6e8da11492

Comments

@sandeep-krishnamurthy @apeforest

@karan6181
Copy link
Contributor

@mxnet-label-bot add [Operator, Performance, pr-awaiting-review]

Copy link
Contributor

@sandeep-krishnamurthy sandeep-krishnamurthy left a comment

Choose a reason for hiding this comment

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

Overall looks good to me.
Please add results seen on CPU/GPU for reference.
Did you run opperf for all operators after this change?

Copy link
Contributor

@sandeep-krishnamurthy sandeep-krishnamurthy left a comment

Choose a reason for hiding this comment

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

Thanks. LGTM

benchmark/opperf/rules/default_params.py Show resolved Hide resolved
Copy link
Contributor

@sandeep-krishnamurthy sandeep-krishnamurthy left a comment

Choose a reason for hiding this comment

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

In the results, I don't see avg forward time for "flip" and "swapaxes" ?

@ChaiBapchya
Copy link
Contributor Author

Ya right. Moreover, functions like
stop_gradient, identity, flatten, broadcast_plus, broadcast_minus also don't have avg_forward_time. Need to probe. Any idea?

@ChaiBapchya
Copy link
Contributor Author

Also, found that sum_axis shows avg_time_forward_sum instead of avg_time_forward_sum_axis

All these operators are aliases and are untracked or wrongly parsed from the profiler output.
Addressing in upcoming commit.

Copy link
Contributor

@sandeep-krishnamurthy sandeep-krishnamurthy left a comment

Choose a reason for hiding this comment

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

Thanks.
LGTM.
Created a feature request to track enhancements we need to make to MXNet operator registry APIs -
#15654

Copy link
Contributor

@larroy larroy left a comment

Choose a reason for hiding this comment

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

LGTM

@sandeep-krishnamurthy sandeep-krishnamurthy merged commit b00bb81 into apache:master Jul 25, 2019
@ChaiBapchya ChaiBapchya deleted the transpose_opperf branch July 28, 2019 05:52
@ChaiBapchya ChaiBapchya mentioned this pull request Aug 2, 2019
3 tasks
anirudhacharya pushed a commit to anirudhacharya/mxnet that referenced this pull request Aug 20, 2019
* add array rearrange operators to opperf

* Trigger notification

* 4d tensor, param support

* new line

* add alias logic
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants