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

Add erfinv operator for calculating inverse error function #13811

Merged
merged 17 commits into from
Jan 22, 2019

Conversation

rondogency
Copy link
Contributor

@rondogency rondogency commented Jan 9, 2019

Description

Add erfinv math operator. This is an extension of the previous error function implementation #13229.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
  • Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
  • Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
  • Code is well-documented:
  • For user-facing API changes, API doc string has been updated.
  • For new C++ functions in header files, their functionalities and arguments are documented.
  • For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
  • Check the API doc at http:https://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

None

Comments

None

@rondogency rondogency requested a review from szha as a code owner January 9, 2019 05:22
@anirudhacharya
Copy link
Member

@mxnet-label-bot add [pr-awaiting-review]

@marcoabreu marcoabreu added the pr-awaiting-review PR is waiting for code review label Jan 9, 2019
Copy link
Member

@eric-haibin-lin eric-haibin-lin left a comment

Choose a reason for hiding this comment

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

@rondogency
Copy link
Contributor Author

@eric-haibin-lin the unix gpu failed on a dropout_perf test. Have you even seen it before? I am a bit struggling to find the root cause of it.

@eric-haibin-lin
Copy link
Member

The failure does not seem related. I've triggered the test again to see if it passes this time

src/operator/contrib/erfinv-inl.h Outdated Show resolved Hide resolved
src/operator/contrib/erfinv-inl.h Outdated Show resolved Hide resolved
Copy link
Member

@eric-haibin-lin eric-haibin-lin left a comment

Choose a reason for hiding this comment

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

Looks good pending one minor comment

tools/license_header.py Outdated Show resolved Hide resolved
@eric-haibin-lin eric-haibin-lin merged commit b86ccf1 into apache:master Jan 22, 2019
jessr92 pushed a commit to jessr92/incubator-mxnet that referenced this pull request Jan 27, 2019
)

* add default behaviour for argmax

* prototype of erfvin

* add test

* gpu support

* Revert "add default behaviour for argmax"

This reverts commit 64e9f1a.

* move erfinv to contrib

* edit copyright

* remove atof

* use std and update license

* add license exclude file

* fix per eric's comments

* change license header
stephenrawls pushed a commit to stephenrawls/incubator-mxnet that referenced this pull request Feb 16, 2019
)

* add default behaviour for argmax

* prototype of erfvin

* add test

* gpu support

* Revert "add default behaviour for argmax"

This reverts commit 64e9f1a.

* move erfinv to contrib

* edit copyright

* remove atof

* use std and update license

* add license exclude file

* fix per eric's comments

* change license header
@TaoLv
Copy link
Member

TaoLv commented Feb 21, 2019

@rondogency Could you take a look at those warnings introduced by this PR? If possible we need get rid of them.

src/operator/tensor/./.././contrib/erfinv-inl.h(79): warning: floating-point value does not fit in required integral type
          detected during:
            instantiation of "DType mxnet::op::mshadow_op::erfinv::Map(DType) [with DType=uint8_t]"
src/operator/tensor/./../mxnet_op.h(479): here
            instantiation of "void mxnet::op::mxnet_op::op_with_req<OP, req>::Map(mxnet::index_t, DType *, const DType *) [with OP=mxnet::op::mshadow_op::erfinv, req=1, DType=uint8_t]"
src/operator/tensor/./../mxnet_op.h(701): here
            instantiation of "void mxnet::op::mxnet_op::mxnet_generic_kernel<OP,Args...>(int, Args...) [with OP=mxnet::op::mxnet_op::op_with_req<mxnet::op::mshadow_op::erfinv, 1>, Args=<uint8_t *, uint8_t *>]"
src/operator/tensor/./../mxnet_op.h(721): here
            instantiation of "void mxnet::op::mxnet_op::Kernel<OP, mxnet::gpu>::Launch(mshadow::Stream<mshadow::gpu> *, int, Args...) [with OP=mxnet::op::mxnet_op::op_with_req<mxnet::op::mshadow_op::erfinv, 1>, Args=<uint8_t *, uint8_t *>]"
src/operator/tensor/./elemwise_unary_op.h(243): here
            instantiation of "void mxnet::op::UnaryOp::Compute<xpu,OP>(const nnvm::NodeAttrs &, const mxnet::OpContext &, const std::vector<mxnet::TBlob, std::allocator<mxnet::TBlob>> &, const std::vector<mxnet::OpReqType, std::allocator<mxnet::OpReqType>> &, const std::vector<mxnet::TBlob, std::allocator<mxnet::TBlob>> &) [with xpu=mxnet::gpu, OP=mxnet::op::mshadow_op::erfinv]"
src/operator/tensor/elemwise_unary_op_basic.cu(67): here

haohuanw pushed a commit to haohuanw/incubator-mxnet that referenced this pull request Jun 23, 2019
)

* add default behaviour for argmax

* prototype of erfvin

* add test

* gpu support

* Revert "add default behaviour for argmax"

This reverts commit 64e9f1a.

* move erfinv to contrib

* edit copyright

* remove atof

* use std and update license

* add license exclude file

* fix per eric's comments

* change license header
Comment on lines +1 to +41
/*
* Copyright (c) 2014 Indiana University
* All rights reserved.
* Written by Prof. Gary L. Pavlis, Dept. of Geol. Sci.,
* Indiana University, Bloomington, IN
* This software is licensed under the New BSD license:
* Redistribution and use in source and binary forms,
* with or without modification, are permitted provided
* that the following conditions are met:
* Redistributions of source code must retain the above
* copyright notice, this list of conditions and the
* following disclaimer.
* Redistributions in binary form must reproduce the
* above copyright notice, this list of conditions and
* the following disclaimer in the documentation and/or
* other materials provided with the distribution.
* Neither the name of Indiana University nor
* the names of its contributors may be used to endorse
* or promote products derived from this software without
* specific prior written permission.
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
* CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED
* WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
* PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
* THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY
* DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
* PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
* USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER
* IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
* NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
* USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*/
/*
* The next function is taken from
* https://github.com/antelopeusersgroup/antelope_contrib/blob/master/lib/location/libgenloc/erfinv.c.
* Output was modified to be inf or -inf when input is 1 or -1.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

@rondogency do you remember where this license came from?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr-awaiting-review PR is waiting for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants