Add mutator and accessor functions for some layers #2121
Conversation
sreenikSS
commented
Dec 21, 2019
sreenikSS
commented
- Add accessor and mutator functions for the padding dimensions of the Convolution Layer.
- Add accessor function for obtaining the input and output dimensions of the LinearNoBias layer.
| @@ -205,7 +205,7 @@ class AtrousConvolution | |||
| //! Modify the output height. | |||
| size_t& OutputHeight() { return outputHeight; } | |||
|
|
|||
| //! Get the input size | |||
| //! Get the input size. | |||
There was a problem hiding this comment.
Haha, was pointed out by @walragatver in a previous discussion.
|
@zoq thanks for approving. I'd be happy if I can get an opinion regarding something I've been thinking about. I might need a few other changes in the mlpack code to make it fully compatible for the model converter. In that case I would have to open another PR (and there would be multiple closed PRs doing the same thing). Instead, would you suggest I make the changes in my branch and To summarize, is opening multiple PRs doing small parts of one big chunk of work considered good practice? |
![mlpack-bot[bot]](https://avatars.githubusercontent.com/in/23934?s=60&v=4){kind=small}
Up to you---it makes it easier to review and get it merged in, but it's also a little more overhead for you. Personally I'm fine with either approach; not sure if others have different opinions. |
|
Agreed, it makes the review process easier, if you are going for multiple PR's it would be great if you could layout the things you like to change, so that we can get an overview. |
