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

_backward_softsign activation is incorrect #10868

Closed
eric-haibin-lin opened this issue May 9, 2018 · 1 comment
Closed

_backward_softsign activation is incorrect #10868

eric-haibin-lin opened this issue May 9, 2018 · 1 comment
Assignees

Comments

@eric-haibin-lin
Copy link
Member

The following test case will fail:

Adding modify the test case in https://github.com/apache/incubator-mxnet/blob/master/tests/python/unittest/test_operator.py#L5867

def test_activation():
    shape=(9, 10)
    dtype_l = [np.float64, np.float32, np.float16]
    rtol_l = [1e-7, 1e-6, 1e-2]
    atol_l = [1e-7, 1e-6, 1e-2]
    rtol_fd = 1e-5
    atol_fd = 1e-6
    num_eps = 1e-6
    unary_ops = {
        'softsign': [lambda x: mx.sym.Activation(x, act_type='softsign'),
                    lambda x: x / (1. + np.abs(x)),
                    lambda x: 1. / np.square(1. + np.abs(x)),

    }
    # Loop over operators
    for name, op in unary_ops.items():
        # Loop over dtype's
        for ind in range(len(dtype_l)):
            dtype = dtype_l[ind]
            rtol = rtol_l[ind]
            atol = atol_l[ind]
            compare_forw_backw_unary_op(
                name, op[0], op[1], op[2], shape, op[3], op[4], rtol, atol,
                dtype)
        # Finite difference testing
        finite_diff_unary_op(
            name, op[0], shape, op[3], op[4], rtol_fd, atol_fd, num_eps)

Reason: For y = softsign(x), the inputs for _backward_softsign and _backward_Activation are different:
_backward_softsign takes (dy, x) as input, backward_Activation takes (dy, y) as input.

@nswamy

@nswamy nswamy self-assigned this May 9, 2018
samskalicky pushed a commit to samskalicky/incubator-mxnet that referenced this issue Jul 19, 2018
problem was that softsign was computed using outputs instead of inputs
added inputs to list, and changed what gets passed into softsign calculation
added softsign test to test_activation function
code reviewed by anirudh
samskalicky pushed a commit to samskalicky/incubator-mxnet that referenced this issue Jul 19, 2018
problem was that softsign was computed using outputs instead of inputs
added inputs to list, and changed what gets passed into softsign calculation
added softsign test to test_activation function
code reviewed by anirudh
**amended to change tab to spaces
anirudh2290 pushed a commit that referenced this issue Jul 20, 2018
* fix for bug #10868: _backward_softsign activation is incorrect
problem was that softsign was computed using outputs instead of inputs
added inputs to list, and changed what gets passed into softsign calculation
added softsign test to test_activation function
code reviewed by anirudh

* fix for bug #10868: _backward_softsign activation is incorrect
problem was that softsign was computed using outputs instead of inputs
added inputs to list, and changed what gets passed into softsign calculation
added softsign test to test_activation function
code reviewed by anirudh
**amended to change tab to spaces

* rerunning the CI build

* fixed size checks for when USE_MKLDNN=ON
@haojin2
Copy link
Contributor

haojin2 commented Jul 20, 2018

@eric-haibin-lin Should be good for closing as the fix was just merged?

KellenSunderland pushed a commit to KellenSunderland/incubator-mxnet that referenced this issue Jul 21, 2018
…pache#11827)

* fix for bug apache#10868: _backward_softsign activation is incorrect
problem was that softsign was computed using outputs instead of inputs
added inputs to list, and changed what gets passed into softsign calculation
added softsign test to test_activation function
code reviewed by anirudh

* fix for bug apache#10868: _backward_softsign activation is incorrect
problem was that softsign was computed using outputs instead of inputs
added inputs to list, and changed what gets passed into softsign calculation
added softsign test to test_activation function
code reviewed by anirudh
**amended to change tab to spaces

* rerunning the CI build

* fixed size checks for when USE_MKLDNN=ON
XinYao1994 pushed a commit to XinYao1994/incubator-mxnet that referenced this issue Aug 29, 2018
…pache#11827)

* fix for bug apache#10868: _backward_softsign activation is incorrect
problem was that softsign was computed using outputs instead of inputs
added inputs to list, and changed what gets passed into softsign calculation
added softsign test to test_activation function
code reviewed by anirudh

* fix for bug apache#10868: _backward_softsign activation is incorrect
problem was that softsign was computed using outputs instead of inputs
added inputs to list, and changed what gets passed into softsign calculation
added softsign test to test_activation function
code reviewed by anirudh
**amended to change tab to spaces

* rerunning the CI build

* fixed size checks for when USE_MKLDNN=ON
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants