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

ListBuilder.layer(...) shouldn't need the index. #3888

Closed
stolsvik opened this Issue Aug 20, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@stolsvik
Copy link
Contributor

stolsvik commented Aug 20, 2017

Currently, when you use the ListBuilder, you need to specify the index for each layer. This is rather error prone when you copy-paste a layer, add a layer, reorganize layers etc.

Since the ListBuilder has full knowledge about the currently added layers, it could just find the current max number, and add the layer "below" that. This can thus be retrofitted straight in, just make an additional layer method w/o the index, have that method find the current max-num and invoke the existing with that number+1.

This would make the fluent API a bit more fluent:

This:

             .list()
            .layer(0, new DenseLayer.Builder()
                    .nOut(1000)
                    .build())
            .layer(1, new DenseLayer.Builder()
                    .nOut(500)
                    .build())
            .layer(2, new DenseLayer.Builder()
                    .nOut(1000)
                    .build())
            .layer(3, new OutputLayer.Builder(LossFunctions.LossFunction.NEGATIVELOGLIKELIHOOD) //create hidden layer
                    .activation(Activation.SOFTMAX)
                    .nOut(numOutputs)
                    .build())
            .setInputType(InputType.feedForward(numInputs))

would become:

            .list()
            .layer(new DenseLayer.Builder()
                    .nOut(1000)
                    .build())
            .layer(new DenseLayer.Builder()
                    .nOut(500)
                    .build())
            .layer(new DenseLayer.Builder()
                    .nOut(1000)
                    .build())
            .layer(new OutputLayer.Builder(LossFunctions.LossFunction.NEGATIVELOGLIKELIHOOD) //create hidden layer
                    .activation(Activation.SOFTMAX)
                    .nOut(numOutputs)
                    .build())
            .setInputType(InputType.feedForward(numInputs))

.. without the problem of having to re-index all the indices if you want to e.g. add a new DropoutLayer at the top..

@AlexDBlack

This comment has been minimized.

Copy link
Member

AlexDBlack commented Aug 22, 2017

@AlexDBlack AlexDBlack closed this Aug 22, 2017

@lock

This comment has been minimized.

Copy link

lock bot commented Sep 25, 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 Sep 25, 2018

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