Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nd4j.gemm(a, b, result, ...) wrong results #6521

Closed
treo opened this issue Oct 4, 2018 · 4 comments · Fixed by #6546

Comments

@treo
Copy link

commented Oct 4, 2018

I ran into an issue where Nd4j.gemm will put results in the wrong order into a target array. If I change the order of the target array from 'c' to 'f' it doesn't improve things. If the target is a view and the original array is in f order, then the result is even more wrong with zeros instead of actual results in half of it.

I would expect that all of the following tests should pass, however 1, 2 and 3 fail, while 4, 5 and 6 pass.

This happens with both the native and cuda backends, and with -beta2 and -SNAPSHOT versions.

 @Test
    public void testGemm1(){
        final INDArray a = Nd4j.rand(3, 4);
        final INDArray b = Nd4j.rand(4, 5);

        final INDArray target = Nd4j.zeros(new int[]{2, 3, 5}, 'f');
        final INDArray view = target.tensorAlongDimension(0, 1, 2);

        Nd4j.gemm(a, b, view, false, false, 1.0, 0.0);

        final INDArray result = a.mmul(b);

        assertEquals(result, view);
    }

    @Test
    public void testGemm2(){
        final INDArray a = Nd4j.rand(4, 3);
        final INDArray b = Nd4j.rand(4, 5);

        final INDArray target = Nd4j.zeros(3, 5);

        Nd4j.gemm(a, b, target, true, false, 1.0, 0.0);

        final INDArray result = a.transpose().mmul(b);

        assertEquals(result, target);
    }

    @Test
    public void testGemm3(){
        final INDArray a = Nd4j.rand(4, 3);
        final INDArray b = Nd4j.rand(4, 5);

        final INDArray target = Nd4j.zeros(new int[]{2, 3, 5}, 'f');
        final INDArray view = target.tensorAlongDimension(0, 1, 2);

        Nd4j.gemm(a, b, view, true, false, 1.0, 0.0);

        final INDArray result = a.transpose().mmul(b);

        assertEquals(result, view);
    }

    @Test
    public void testGemm4() {
        final INDArray a = Nd4j.rand(4, 3);
        final INDArray b = Nd4j.rand(4, 5);

        final INDArray result = a.transpose().mmul(b);
        final INDArray result2 = Nd4j.gemm(a, b, true, false);

        assertEquals(result, result2);
    }

    @Test
    public void testGemm5(){
        final INDArray a = Nd4j.rand(4, 3);
        final INDArray b = Nd4j.rand(4, 5);

        final INDArray target = Nd4j.zeros(new int[]{2, 3, 5}, 'f');
        final INDArray view = target.tensorAlongDimension(0, 1, 2);

        a.transpose().mmuli(b, view);

        final INDArray result = a.transpose().mmul(b);

        assertEquals(result, view);
    }

    @Test
    public void testGemm6(){
        final INDArray a = Nd4j.rand(4, 3);
        final INDArray b = Nd4j.rand(4, 5);

        final INDArray target = Nd4j.zeros(new int[]{2, 3, 5}, 'c');
        final INDArray view = target.tensorAlongDimension(0, 1, 2);

        a.transpose().mmuli(b, view);

        final INDArray result = a.transpose().mmul(b);

        assertEquals(result, view);
    }
@raver119

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2018

As i remember, Nd4j.gemm supposed to be pretty much raw blas call, so it's not aware of views etc.

@AlexDBlack

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2018

Right, so it's a validation issue. We should throw an exception on invalid input (views, c order, etc).

@treo

This comment has been minimized.

Copy link
Author

commented Oct 4, 2018

So Test 1 and 3 fail because they are views, and Test 2 fails because the target is in c order?

@AlexDBlack AlexDBlack self-assigned this Oct 8, 2018

AlexDBlack added a commit that referenced this issue Oct 8, 2018
AlexDBlack added a commit that referenced this issue Oct 10, 2018
DL4J/ND4J Fixes (#6546)
* #6539 Handle 0 gradient case for gradient normalization

* #6521 Nd4j.gemm validation

* #6543 View/order checks for BaseNDArray.mmuli()

* #6542 mmuli shape validation

* #6545 Require scalars, vectors, or same shape for INDArray.assign()

* #6520 Fix setLearningRate(double) for the no updater state (SGD, etc) case

* #6490 Fix StackVertex NPE with some masking cases

* Cnn3DLossLayer. Typo in RecordReaderMultiDataSetIteratorTest.

* Small fix

* Cnn3DLossLayer gradient checks (not yet passing)

* Cnn3dLossLayer masking + test fixes

* Extra tests, CNN3D tweaks

* Fix Conv3d layer support for NDHWC data format

* Fix Cnn3DLossLayer

* Allow size 1 dimensions in assign shape check

* Minor test fixes

* Fix RollAxis; other tweaks
@lock

This comment has been minimized.

Copy link

commented Nov 9, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Nov 9, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants
You can’t perform that action at this time.