hollance / Forge

A neural network toolkit for Metal
MIT License
1.27k stars 172 forks source link

Use more sensible defaults #1

Closed tLewisII closed 6 years ago

tLewisII commented 7 years ago

There are a few places where I think adding some defaults in constructors would be beneficial/sensible.

Stuff like inflightBuffers and kernel in Convolution and Pooling layers could have defaults that would reduce repetition and clean up model construction.

On the flip side, perhaps some people might not notice the default params and it could lead to errors.

Thoughts?

hollance commented 7 years ago

What would you suggest for these defaults? I'm open to the suggestion but one person's defaults may not be reasonable to another person.

For example, I chose padding: true as the default for Convolution layers (makes sense to me) but Keras uses padding: 'VALID' as the default, which is the opposite of what Forge uses. This already lead to confusion with a user of Forge.

So I'm all for reasonable defaults but I'm not sure yet what "reasonable" means here. ;-)

One possibility would be something like this:

let model = Model
    --> SetDefaults(kernel: (3, 3), padding: true)
    --> Convolution()
    --> and so on
tLewisII commented 7 years ago

I was thinking the following:

So not a huge amount of changes really.

hollance commented 7 years ago

I'll have to think some more about this. You're right that 3x3 convolution seems to be very common now, but there is an argument to be made for keeping certain things (like the kernel sizes) explicitly specified.