You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@madlib.apache.org by iyerr3 <gi...@git.apache.org> on 2018/03/28 05:18:00 UTC
[GitHub] madlib pull request #251: MLP: Simplify initialization of model coefficients
GitHub user iyerr3 opened a pull request:
https://github.com/apache/madlib/pull/251
MLP: Simplify initialization of model coefficients
Changes:
1. Model initialization now happens in the C++ code instead of being
passed via Python.
2. Warm start coefficients are still passed from Python. These
coefficients are copied into the model. The vectorized form of the copy
raises an Eigen assertion when used with a MappedMatrix, possibly due
to the actual block size not being available via the Map.
3. The distinction between a "TaskState" and an "AlgoState" within a
State type has been removed in the MLPMiniBatchState. This demarcation
seemed to be confusing and didn't provide much abstraction.
Closes #251
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/iyerr3/incubator-madlib bugfix/minibatch_init_debug
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/madlib/pull/251.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #251
----
----
---
[GitHub] madlib pull request #251: MLP: Simplify initialization of model coefficients
Posted by njayaram2 <gi...@git.apache.org>.
Github user njayaram2 commented on a diff in the pull request:
https://github.com/apache/madlib/pull/251#discussion_r178204067
--- Diff: src/modules/convex/mlp_igd.cpp ---
@@ -98,23 +98,28 @@ mlp_igd_transition::run(AnyType &args) {
double is_classification_double = (double) is_classification;
double activation_double = (double) activation;
- MappedColumnVector coeff = args[10].getAs<MappedColumnVector>();
- state.task.model.rebind(&is_classification_double,&activation_double,
- &coeff.data()[0], numberOfStages,
- &numbersOfUnits[0]);
-
- // state.task.model.is_classification =
- // static_cast<double>(is_classification);
- // state.task.model.activation = static_cast<double>(activation);
- // MappedColumnVector initial_coeff = args[10].getAs<MappedColumnVector>();
- // // copy initial_coeff into the model
- // Index fan_in, fan_out, layer_start = 0;
- // for (size_t k = 0; k < numberOfStages; ++k){
- // fan_in = numbersOfUnits[k];
- // fan_out = numbersOfUnits[k+1];
- // state.task.model.u[k] << initial_coeff.segment(layer_start, (fan_in+1)*fan_out);
- // layer_start = (fan_in + 1) * fan_out;
- // }
+ if (!args[10].isNull()){
+ // initial coefficients are provided
+ MappedColumnVector warm_start_coeff = args[10].getAs<MappedColumnVector>();
+
+ // copy warm start into the task model
+ // state.reset() ensures algo.incrModel is copied from task.model
+ Index layer_start = 0;
+ for (size_t k = 0; k < numberOfStages; ++k){
+ for (size_t j=0; j < state.task.model.u[k].cols(); ++j){
+ for (size_t i=0; i < state.task.model.u[k].rows(); ++i){
+ state.task.model.u[k](i, j) = warm_start_coeff(
+ layer_start + j * state.task.model.u[k].rows() + i);
+ }
+ }
+ layer_start = state.task.model.u[k].rows() * state.task.model.u[k].cols();
+ }
+ } else {
+ // initialize the model with appropriate coefficients
+ state.task.model.initialize(
--- End diff --
This is more of a doubt than a comment I guess. Should this be `state.algo.incrModel` instead?
The reason I thought so was that `MLPIGDAlgorithm::transition(state, tuple);` uses the model in algo instead of the one task.
---
[GitHub] madlib issue #251: MLP: Simplify initialization of model coefficients
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/madlib/pull/251
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/madlib-pr-build/414/
---
[GitHub] madlib issue #251: MLP: Simplify initialization of model coefficients
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/madlib/pull/251
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/madlib-pr-build/409/
---
[GitHub] madlib issue #251: MLP: Simplify initialization of model coefficients
Posted by fmcquillan99 <gi...@git.apache.org>.
Github user fmcquillan99 commented on the issue:
https://github.com/apache/madlib/pull/251
Using the data set from
http://madlib.apache.org/docs/latest/group__grp__nn.html#example
the warm start seems to be functioning OK in the sense that it is picking up where it left off:
```
DROP TABLE IF EXISTS mlp_model, mlp_model_summary, mlp_model_standardization;
-- Set seed so results are reproducible
SELECT setseed(0);
SELECT madlib.mlp_classification(
'iris_data', -- Source table
'mlp_model', -- Destination table
'attributes', -- Input features
'class_text', -- Label
ARRAY[5], -- Number of units per layer
'learning_rate_init=0.003,
n_iterations=10,
tolerance=0', -- Optimizer params
'tanh', -- Activation function
NULL, -- Default weight (1)
FALSE, -- No warm start
TRUE -- Not verbose
);
```
produces
```
INFO: Iteration: 2, Loss: <1.52802997435>
INFO: Iteration: 3, Loss: <1.48759899134>
INFO: Iteration: 4, Loss: <1.45013924747>
INFO: Iteration: 5, Loss: <1.41474941405>
INFO: Iteration: 6, Loss: <1.38065258666>
INFO: Iteration: 7, Loss: <1.34717834047>
INFO: Iteration: 8, Loss: <1.31375187164>
INFO: Iteration: 9, Loss: <1.27988891477>
```
Multiple calls of
```
SELECT madlib.mlp_classification(
'iris_data', -- Source table
'mlp_model', -- Destination table
'attributes', -- Input features
'class_text', -- Label
ARRAY[5], -- Number of units per layer
'learning_rate_init=0.003,
n_iterations=10,
tolerance=0', -- Optimizer params
'tanh', -- Activation function
NULL, -- Default weight (1)
TRUE, -- Warm start
TRUE -- Not verbose
);
```
produces
```
INFO: Iteration: 2, Loss: <1.17220527126>
INFO: Iteration: 3, Loss: <1.1336000279>
INFO: Iteration: 4, Loss: <1.09355192251>
INFO: Iteration: 5, Loss: <1.05216070965>
INFO: Iteration: 6, Loss: <1.00962024929>
INFO: Iteration: 7, Loss: <0.966205768022>
INFO: Iteration: 8, Loss: <0.922255641274>
INFO: Iteration: 9, Loss: <0.878149011376>
```
```
INFO: Iteration: 2, Loss: <0.748780710824>
INFO: Iteration: 3, Loss: <0.707821562402>
INFO: Iteration: 4, Loss: <0.668421333885>
INFO: Iteration: 5, Loss: <0.630777478881>
INFO: Iteration: 6, Loss: <0.595026837568>
INFO: Iteration: 7, Loss: <0.56124977596>
INFO: Iteration: 8, Loss: <0.529476974538>
INFO: Iteration: 9, Loss: <0.499697614318>
```
INFO: Iteration: 2, Loss: <0.421765880284>
INFO: Iteration: 3, Loss: <0.399310193088>
INFO: Iteration: 4, Loss: <0.378448795893>
INFO: Iteration: 5, Loss: <0.359075938873>
INFO: Iteration: 6, Loss: <0.341086602199>
INFO: Iteration: 7, Loss: <0.324378642438>
INFO: Iteration: 8, Loss: <0.308854261119>
INFO: Iteration: 9, Loss: <0.294420938642>
```
INFO: Iteration: 2, Loss: <0.256830594513>
INFO: Iteration: 3, Loss: <0.245954682329>
INFO: Iteration: 4, Loss: <0.235795731167>
INFO: Iteration: 5, Loss: <0.226295883531>
INFO: Iteration: 6, Loss: <0.217402237653>
INFO: Iteration: 7, Loss: <0.209066485211>
INFO: Iteration: 8, Loss: <0.201244549645>
INFO: Iteration: 9, Loss: <0.193896234701>
```
If I run with 50 iterations at one time (no warm start), it ends up with
```
INFO: Iteration: 49, Loss: <0.173285421454>
```
which is in the ballpark of the 5 separate runs above, so I think this looks OK.
It would be better if the loss for the 1st and last iterations was printed out in verbose mode.
---
[GitHub] madlib issue #251: MLP: Simplify initialization of model coefficients
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/madlib/pull/251
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/madlib-pr-build/417/
---
[GitHub] madlib pull request #251: MLP: Simplify initialization of model coefficients
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/madlib/pull/251
---
[GitHub] madlib issue #251: MLP: Simplify initialization of model coefficients
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/madlib/pull/251
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/madlib-pr-build/410/
---
[GitHub] madlib issue #251: MLP: Simplify initialization of model coefficients
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/madlib/pull/251
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/madlib-pr-build/415/
---
[GitHub] madlib pull request #251: MLP: Simplify initialization of model coefficients
Posted by iyerr3 <gi...@git.apache.org>.
Github user iyerr3 commented on a diff in the pull request:
https://github.com/apache/madlib/pull/251#discussion_r178204767
--- Diff: src/modules/convex/mlp_igd.cpp ---
@@ -98,23 +98,28 @@ mlp_igd_transition::run(AnyType &args) {
double is_classification_double = (double) is_classification;
double activation_double = (double) activation;
- MappedColumnVector coeff = args[10].getAs<MappedColumnVector>();
- state.task.model.rebind(&is_classification_double,&activation_double,
- &coeff.data()[0], numberOfStages,
- &numbersOfUnits[0]);
-
- // state.task.model.is_classification =
- // static_cast<double>(is_classification);
- // state.task.model.activation = static_cast<double>(activation);
- // MappedColumnVector initial_coeff = args[10].getAs<MappedColumnVector>();
- // // copy initial_coeff into the model
- // Index fan_in, fan_out, layer_start = 0;
- // for (size_t k = 0; k < numberOfStages; ++k){
- // fan_in = numbersOfUnits[k];
- // fan_out = numbersOfUnits[k+1];
- // state.task.model.u[k] << initial_coeff.segment(layer_start, (fan_in+1)*fan_out);
- // layer_start = (fan_in + 1) * fan_out;
- // }
+ if (!args[10].isNull()){
+ // initial coefficients are provided
+ MappedColumnVector warm_start_coeff = args[10].getAs<MappedColumnVector>();
+
+ // copy warm start into the task model
+ // state.reset() ensures algo.incrModel is copied from task.model
+ Index layer_start = 0;
+ for (size_t k = 0; k < numberOfStages; ++k){
+ for (size_t j=0; j < state.task.model.u[k].cols(); ++j){
+ for (size_t i=0; i < state.task.model.u[k].rows(); ++i){
+ state.task.model.u[k](i, j) = warm_start_coeff(
+ layer_start + j * state.task.model.u[k].rows() + i);
+ }
+ }
+ layer_start = state.task.model.u[k].rows() * state.task.model.u[k].cols();
+ }
+ } else {
+ // initialize the model with appropriate coefficients
+ state.task.model.initialize(
--- End diff --
The comment in line 106 is actually for both the warm start copy and the initialize. Basically `state.reset()` in line 125 is copying `task.model` to `algo.incrModel`.
---
[GitHub] madlib issue #251: MLP: Simplify initialization of model coefficients
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/madlib/pull/251
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/madlib-pr-build/411/
---