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

[v1.9.x] modify erfinv implementation based on scipy #20517

Merged
merged 5 commits into from
Aug 14, 2021
Merged

[v1.9.x] modify erfinv implementation based on scipy #20517

merged 5 commits into from
Aug 14, 2021

Conversation

mseth10
Copy link
Contributor

@mseth10 mseth10 commented Aug 12, 2021

Description

Modify erfinv implementation based on SciPy's implementation. This fixes #20511

Performance Analysis

Ran a simple python script twice

import mxnet as mx
import time

y = mx.nd.random.uniform(-1, 1, shape=(100,100))
start = time.time()
for i in range(1000000):
    x= mx.ndarray.erfinv(y)
    x.wait_to_read()
end = time.time()

print(end-start)

The earlier implementation took 51.19 secs and 53.06 secs.
With this PR changes, it takes 46.29 secs and 41.04 secs.

So, the new implementation is faster.

@mxnet-bot
Copy link

Hey @mseth10 , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:

  • To trigger all jobs: @mxnet-bot run ci [all]
  • To trigger specific jobs: @mxnet-bot run ci [job1, job2]

CI supported jobs: [centos-gpu, miscellaneous, unix-cpu, unix-gpu, windows-gpu, edge, website, sanity, clang, windows-cpu, centos-cpu]

Note:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.

@mseth10 mseth10 added the pr-awaiting-testing PR is reviewed and waiting CI build and test label Aug 12, 2021
@mseth10 mseth10 self-assigned this Aug 12, 2021
@mseth10
Copy link
Contributor Author

mseth10 commented Aug 12, 2021

Is this file rightly placed in rat-excludes and tools/license_header.py ? @josephevans @leezu
https://github.com/apache/incubator-mxnet/blob/v1.9.x/rat-excludes#L124
https://github.com/apache/incubator-mxnet/blob/v1.9.x/tools/license_header.py#L108

@mseth10 mseth10 added pr-work-in-progress PR is still work in progress and removed pr-awaiting-testing PR is reviewed and waiting CI build and test labels Aug 12, 2021
@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress and removed pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test labels Aug 12, 2021
@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress and removed pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test labels Aug 12, 2021
@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress and removed pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test labels Aug 13, 2021
@mseth10
Copy link
Contributor Author

mseth10 commented Aug 13, 2021

@mxnet-bot run ci [unix-cpu ]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-cpu]

@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test and removed pr-work-in-progress PR is still work in progress labels Aug 13, 2021
@mseth10
Copy link
Contributor Author

mseth10 commented Aug 13, 2021 •
edited
Loading

@mxnet-bot run ci [unix-gpu ]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-gpu]

@mseth10 mseth10 added pr-work-in-progress PR is still work in progress and removed pr-awaiting-testing PR is reviewed and waiting CI build and test labels Aug 13, 2021
@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-gpu]

@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress and removed pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test labels Aug 13, 2021
@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-gpu]

@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test and removed pr-work-in-progress PR is still work in progress labels Aug 13, 2021
josephevans
josephevans approved these changes Aug 14, 2021
View reviewed changes
Copy link
Contributor

@josephevans josephevans left a comment

Choose a reason for hiding this comment

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

Thanks for refactoring this! LGTM!

@mseth10 mseth10 added pr-awaiting-merge Review and CI is complete. Ready to Merge and removed pr-awaiting-testing PR is reviewed and waiting CI build and test labels Aug 14, 2021
@mseth10 mseth10 merged commit 7862c00 into apache:v1.9.x Aug 14, 2021
@mseth10 mseth10 mentioned this pull request Aug 16, 2021
Closed
mseth10 added a commit to mseth10/incubator-mxnet that referenced this pull request Aug 23, 2021
@mseth10
[v1.9.x] modify erfinv implementation based on scipy (apache#20517)
2930776 
* modify erfinv implementation based on scipy

* fix lint

* fix lint

* fix host/device gpu error

* fix flag
@mseth10 mseth10 mentioned this pull request Aug 23, 2021
Merged
mseth10 added a commit to mseth10/incubator-mxnet that referenced this pull request Aug 23, 2021
@mseth10
[v1.9.x] modify erfinv implementation based on scipy (apache#20517)
37d18fb 
* modify erfinv implementation based on scipy

* fix lint

* fix lint

* fix host/device gpu error

* fix flag
@mseth10 mseth10 mentioned this pull request Aug 23, 2021
Merged
mseth10 added a commit that referenced this pull request Aug 25, 2021
@mseth10
[v1.9.x] modify erfinv implementation based on scipy (#20517) (#20551)
a7281c6 
* modify erfinv implementation based on scipy

* fix lint

* fix lint

* fix host/device gpu error

* fix flag
mseth10 added a commit that referenced this pull request Aug 25, 2021
@mseth10
[v1.9.x] modify erfinv implementation based on scipy (#20517) (#20550)
da38e28 
* modify erfinv implementation based on scipy

* fix lint

* fix lint

* fix host/device gpu error

* fix flag
KexinFeng pushed a commit to KexinFeng/incubator-mxnet that referenced this pull request Aug 27, 2021
@mseth10 @KexinFeng
[v1.9.x] modify erfinv implementation based on scipy (apache#20517) (a…
94d0eff 
…pache#20550)

* modify erfinv implementation based on scipy

* fix lint

* fix lint

* fix host/device gpu error

* fix flag
bgawrych pushed a commit to bgawrych/incubator-mxnet that referenced this pull request Aug 31, 2021
@mseth10 @bgawrych
[v1.9.x] modify erfinv implementation based on scipy (apache#20517) (a…
2c07f4b 
…pache#20550)

* modify erfinv implementation based on scipy

* fix lint

* fix lint

* fix host/device gpu error

* fix flag
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@josephevans josephevans josephevans approved these changes

@leezu leezu Awaiting requested review from leezu

@szha szha Awaiting requested review from szha

Assignees

@mseth10 mseth10

Labels
pr-awaiting-merge Review and CI is complete. Ready to Merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mseth10 @mxnet-bot @josephevans