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

[OpPerf] Handle positional arguments#15761

Merged
roywei merged 11 commits intoapache:masterfrom
ChaiBapchya:arg_handle_opperf
Aug 19, 2019
Merged

[OpPerf] Handle positional arguments#15761
roywei merged 11 commits intoapache:masterfrom
ChaiBapchya:arg_handle_opperf

Conversation

@ChaiBapchya
Copy link
Contributor

@ChaiBapchya ChaiBapchya commented Aug 6, 2019

Description

Fixes #15735
Currently, OpPerf utility only takes keyworded arguments **kwargs as argument to run the profiler. It is passed as dictionary value to "inputs"

However, certain functions expect positional arguments (*args) instead of keyworded arguments (**kwargs).

This PR handles that case.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Code is well-documented:
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • Handle positional args

Comments

Tested it with
a. Clone and checkout this branch
b. Build the repo
c. Test both the cases

>>> from benchmark.opperf.utils.benchmark_utils import run_performance_test
>>> from mxnet import nd
>>> import mxnet as mx

Without positional arguments

>>> run_performance_test(nd.add,run_backward=True, dtype='float32', ctx=mx.cpu(),inputs=[{"lhs":(2,2),"rhs":(2,2)}],warmup=10, runs=25)
INFO:root:Begin Benchmark - add
INFO:root:Complete Benchmark - add
[{'add': [{'avg_time_forward_add': 0.0701, 'max_storage_mem_alloc_cpu/0': 0.008, 'avg_time_backward_add': 0.055, 'inputs': {'lhs': (2, 2), 'rhs': (2, 2)}}]}]

With positional arguments

>>> run_performance_test(nd.squeeze,run_backward=True, dtype='float32', ctx=mx.cpu(),inputs=[{"args":(10,1,2)}],warmup=10, runs=25)
INFO:root:Begin Benchmark - squeeze
INFO:root:Complete Benchmark - squeeze
[{'squeeze': [{'avg_time_forward_squeeze': 0.0412, 'max_storage_mem_alloc_cpu/0': 0.04, 'avg_time_backward_squeeze': 0.0303, 'inputs': {'args': (10, 1, 2)}}]}]

Copy link
Contributor

@apeforest apeforest left a comment

Choose a reason for hiding this comment

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

LGTM after fixing the indent

@roywei roywei merged commit c50bfe2 into apache:master Aug 19, 2019
@ChaiBapchya ChaiBapchya deleted the arg_handle_opperf branch August 19, 2019 14:40
anirudhacharya pushed a commit to anirudhacharya/mxnet that referenced this pull request Aug 20, 2019
* handle args

* intermediate, error with getting 2 values for data param for other ops

* handle args

* None type issue

* try

* indent fix

* lint fix

* Trigger notification

* Trigger notification bcoz validation/edge passed but shows pending
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Arg name inconsistencies

3 participants