[SYSTEMML-540] [SYSTEMML-515] Allow an expression for sparsity #351
Conversation
- This PR improves the performance of dropout
|
Please also fix it for MR - you can simply use instruction patching as done for rows and cols. |
There was a problem hiding this comment.
Thanks, @niketanpansare for adding this functionality! I added a couple of comments to dropout.dml, but otherwise this will be great.
| @@ -42,10 +42,7 @@ forward = function(matrix[double] X, double p, int seed) | |||
| * - out: Ouptuts, of same shape as X. | |||
| * - mask: Dropout mask used to compute the output. | |||
| */ | |||
| if (seed == -1) { | |||
| seed = as.integer(floor(as.scalar(rand(rows=1, cols=1, min=1, max=100000)))) | |||
| } | |||
There was a problem hiding this comment.
These three lines need to be added back so that a user can indicate that the dropout layer should use a random seed.
| if (seed == -1) { | ||
| seed = as.integer(floor(as.scalar(rand(rows=1, cols=1, min=1, max=100000)))) | ||
| } | ||
| mask = rand(rows=nrow(X), cols=ncol(X), min=0, max=1, seed=seed) <= p |
There was a problem hiding this comment.
Can we add a comment here explaining how we are creating the mask with the sparsity flag, since that is not a common construct? I.e. add the following comment:
# Normally, we might use something like
# `mask = rand(rows=nrow(X), cols=ncol(X), min=0, max=1, seed=seed) <= p`
# to create a dropout mask. Fortunately, SystemML has a `sparsity` parameter on
# the `rand` function that allows use to create a mask directly.
mask = rand(rows=nrow(X), cols=ncol(X), min=1, max=1, sparsity=p, seed=seed)
| // if(!(getVarParam(RAND_SPARSITY) instanceof DoubleIdentifier || getVarParam(RAND_SPARSITY) instanceof IntIdentifier | ||
| // || getVarParam(RAND_SPARSITY) instanceof DataIdentifier)) { | ||
| // raiseValidateError("for Rand statement " + RAND_SPARSITY + " has incorrect value type", conditional); | ||
| // } |
There was a problem hiding this comment.
Could we just remove this code?
|
@mboehm7 @dusenberrymw Incorporated your comments, please review it. @dusenberrymw Instead of generating random seed in DML, we can let rand chose a random seed instead by not providing it an argument :) |
There was a problem hiding this comment.
Great! I just ran the SystemML-NN tests as well, and everything passes still.
LGTM.
|
The integration tests passed as well. |
- This PR also improves the performance of dropout. Closes apache#351.
@bertholdreinwald @mboehm7 @dusenberrymw