[mlpack-git] [mlpack] Fix drop out layer (#463)

[Ryan Curtin]

I've gone back and forth on this many times.  In this particular case it seems like the fastest code would result by a `void SetRatio(double ratioValue)` function instead of a `double& Ratio()` function, since some post-processing of the value needs to be done.  I'd suggest the function name `void Ratio(double ratio)` instead of `SetRatio()`, though, to be more consistent with the other mlpack code that has functions like this (I think this is also done in `GaussianKernel` and a few of the other kernels).

It's usually a subjective call to decide whether to go with a `void Parameter(type parameter)` or a `parameter& Parameter()` function, and I don't think there's a one-size-fits-all solution.  For instance, working with matrices means that something like `void Matrix(mat& matrix)` will be very slow.  Now that we use C++11, we can write `void Matrix(mat&& matrix)`, but this still can make expressions very awkward; consider adding 1 to the diagonals of the matrix.  That'll give something like... `Matrix(std::move(Matrix()) + arma::eye<arma::mat>(Matrix().n_rows, Matrix().n_cols)))`, but if you were able to work directly on the matrix you could just write `Matrix() += arma::eye<arma::mat>(Matrix().n_rows, Matrix().n_cols)`.

I think that in this case, moving the calculation of `scale` outside of `Forward()` may not actually give any noticeable runtime improvement, since the vast majority of the work spent in `Forward()` will be in the matrix operations.  A couple other thoughts in that vein: creating the `mask` object is going to take some amount of memory; it may be better to sequentially generate random numbers in a single loop over the input.  Also, if `deterministic` is true and `rescale` is true, you have to pass through the input matrix twice: once for `output = input` and once for `output *= scale`.  I think it will be faster to do this:

```
if (deterministic)
{
  if (!rescale)
    output = input;
  else
    output = input * scale;
}
```

Of course, it's probably worth running a quick test on either of those ideas to see if they're worth implementing or actually give a runtime improvement.

---
Reply to this email directly or view it on GitHub:
https://github.com/mlpack/mlpack/pull/463#issuecomment-151167047
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mailman.cc.gatech.edu/pipermail/mlpack-git/attachments/20151026/677d3194/attachment.html>