JohnLangford/vowpal_wabbit — pull request #191 by agrubb (2013-11-14 21:01:53+00:00)
Title: Fix bug in handling of feature_mask

These two commits fix two (potential) bugs with the way the feature_mask is loaded.

The first handles an issue with the header values for feature_mask overriding those for the initial_regressor or the defaults set in `parse_args.cc`. For an example where this crops up, consider the simple, but somewhat convoluted example (run from `vw/test/`):

```
$ vw -k --oaa 10 -c --passes 10 train-sets/multiclass --holdout_off --l1 0.003 -f mask.model
[...]
$ vw -k --feature_mask mask.model --oaa 10 -c --passes 10 train-sets/multiclass --holdout_off -f final.model
[...]
$ vw -i final.model -t -c --passes 10 train-sets/multiclass --holdout_off
only testing
terminate called after throwing an instance of 'boost::exception_detail::clone_impl >'
what(): multiple occurrences
Aborted (core dumped)
```

In the options section in `final.model` the option `--oaa 10` appears twice, causing the exception. This is due to the fact that the `vm_file` options are parsed and dealt with before the feature mask file, and so the OAA logic does not think the `oaa` option was present in the regressor file, causing the option to be added twice to the final regressor file.

The second bug is basically the same issue but related to the weights loaded from the feature mask file. Instead of using any weights loaded from the initial regressor, or the default weights of 0.0, the feature mask weights are used instead, since they are loaded last and overwrite any previous values. It's not clear if this is intended or not. My fix for this unfortunately loads the initial regressor file twice, but the alternative seems like it would be messy, and involve adding a lot of logic to `parse_args.cc` that's currently contained in `parse_regressor.cc`.

The final commit just changes the feature mask test to reflect the fact that the weights are started at 0 instead of the values in the feature mask.

Would be happy to discuss better ways to handle the issue if these changes aren't appropriate. The first bug seems more serious, while the second is less serious but seems different from the intended behavior.