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

[OpPerf] Handle positional arguments #15761

Merged
merged 11 commits into from
Aug 19, 2019
Merged

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