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

Add Merge Layer Test Cases #276

Open
therealansh opened this issue Nov 10, 2021 · 6 comments · May be fixed by #313
Open

Add Merge Layer Test Cases #276

therealansh opened this issue Nov 10, 2021 · 6 comments · May be fixed by #313
Assignees

Comments

@therealansh
Copy link
Contributor

KotlinDL is right now does not have Merge Layer test cases like Add, Concatenate, Multiply and so on. I guess addition of test cases will be great. Also what do you think about refactoring and organising the layer test cases directory @zaleslaw?

@zaleslaw
Copy link
Collaborator

Yes, a lot of tests are missed for many layers. It will be great to add them to the test suite

@zaleslaw zaleslaw added this to the 0.4 milestone Nov 10, 2021
@therealansh
Copy link
Contributor Author

Hey @zaleslaw, would it be nice if I put Merge tests in one file as the ReshapeTest looks tidy but what are your thoughts?

@zaleslaw
Copy link
Collaborator

Yes, put it into one file

@therealansh
Copy link
Contributor Author

Hey @zaleslaw , have a look at this test case, this is from tf official doc yet it fails, AFAIK the assertLayerOutputIsCorrect checks the content of the output as well and Average() should produce the same effect as given in the doc. Yet the test fails and when i tried to debug it it just returns the [x1,x2] as result.

@Test
    fun average(){
        val x1 = Array(2){FloatArray(2){1f} }
        val x2 = Array(2){FloatArray(2){0f} }
        val input = arrayOf(x1,x2)
        val expected = Array(2){FloatArray(2){0.5f} }
        assertLayerOutputIsCorrect(Average(), input,expected)
    }

@zaleslaw
Copy link
Collaborator

zaleslaw commented Nov 22, 2021

Thanks for the note @therealansh ! Probably the Average layer could be implemented with a bug (low probability), or you could find something else (high probability) that influences on the output

@therealansh
Copy link
Contributor Author

Hey @zaleslaw, I guess the bug is in LayerTest.kt. Since, Merge layer abstraction has a overloading of forward function, i guess we need overload this as well getLayerOutputOp() as it takes Array<*> which returns a Operand rather than List<Operand<Float>> I guess this can be the only reason for it. Otherwise the Average layer is implemented correctly.

@therealansh therealansh linked a pull request Dec 17, 2021 that will close this issue
8 tasks
@zaleslaw zaleslaw modified the milestones: 0.4, 0.5 Feb 16, 2022
@ermolenkodev ermolenkodev removed this from the 0.5 milestone Sep 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants