Summary:
Main changes:
1. Move reader creation to Brew in order to be consistent and avoid a wild use of param_init_net
2. Use optimizers for training function, avoid manual optimizer construction
3. Add MLP mode (a default)
4. Fix a bunch of too verbose comments and add a bit of new explanations
Closes https://github.com/caffe2/caffe2/pull/1760
Differential Revision: D6749059
Pulled By: salexspb
fbshipit-source-id: 9dfbbb2d9772a74a0300c2e404a92e791f7cc593
Summary:
Caffe2 user was confused when model.TensorProtosDBINput([reader]) did not work. This is because of this outdated model helper function, that ignored the input blobs.
Added assertion to enforce correct usage. I did not want to make this work with reader input as well, since this probably should not be used anyway.
Reviewed By: amanrajdce
Differential Revision: D6380326
fbshipit-source-id: 6a50c2861f7f58c06cbfe3e86bde0f17a2b443cb
Summary:
- hasattr is misbehaving in python 3
- python2: `This is implemented by calling getattr(object, name) and seeing whether it raises an exception or not`
- python3: `This is implemented by calling getattr(object, name) and seeing whether it raises an AttributeError or not.`
Reviewed By: azzolini
Differential Revision: D5973797
fbshipit-source-id: 0b6a413e6ebacd9bdd197c46feab256ab383ace2
Summary: Before this diff RNNOp was using TextFormat for representing steps. This diff is changing RNNOp to prefer NetDef argument instead. To be backward compatible it supports TextFormat for existing models, though we can compile RNNs without TextFormat as well.
Reviewed By: salexspb
Differential Revision: D5949330
fbshipit-source-id: 9336a8f5ccf30ad8d8e3a7067b9437e1704b1c9f
Summary:
The renames were only being applied to the main net, if step_net has an
external input that is part of renames, running the model would fail with 'blob
not found in workspace' error.
Differential Revision: D5511953
fbshipit-source-id: ba262a094c3263978dfe173f2cab00301131b57f
Summary: the init method should also make _parameters_info shared between self and param_model, since params is shared. Otherwise it can cause a inconsistence between _parameters_info and params. Examples of using param_model can be find in rnn_cell.py.
Reviewed By: kennyhorror
Differential Revision: D5405327
fbshipit-source-id: ca8079058e898f529906452163cda234cb30a7df
Summary: this diff adds optimizer into param_info, and the associated implementations for modelhelper and brew to set optimizer for each individual parameter.
Reviewed By: kennyhorror
Differential Revision: D5385432
fbshipit-source-id: 5d682f9d1ab077e04a5d76a24d71470f4e64fc92
Summary: GetComputedParams tests namescopes with equality while GetParams tests with a prefix. Switching GetComputedParams to also use a prefix so that both functions have similar usages.
Reviewed By: akyrola
Differential Revision: D5389816
fbshipit-source-id: 0e43e4b491fccbad3b855b6b735dc2b91d7626c9
Summary: I accidentaly noticed that we were calling the non-CUDNN version of Transpose with attention, and it is super slow. This broke when rnn_cell was changed to use ModelHelper instead of CNNModelHelper in D5062963, but calls to transpose were not "brewed".
Reviewed By: jamesr66a
Differential Revision: D5264248
fbshipit-source-id: b61494ae210f34597245f1195d20547f5b5cd8b5
Summary: Also fixed a small bug in ModelHelper constructor
Reviewed By: harouwu
Differential Revision: D5246799
fbshipit-source-id: 3719ca078f0e2b5e463fc93da9c8215f5583bd9a
Summary:
We need to support RNNs explicitly in ExtractPredictorNet, because they store sub-nets as strings in special arguments. When netdef argument arrive, we can generalize this a bit.
Added a test under rnn_cell_test to test that extracting an LSTM predictor net works correctly and sets the device option properly for the step net ops.
Reviewed By: yqwangustc
Differential Revision: D5236334
fbshipit-source-id: cd653427f8c440a14d94195a532d18276f94749a
Summary:
This diff is fixing fetching of the parameters in the global namescope. Earlier
diff that have switched to '' have introduced this bug.
Reviewed By: dzhulgakov
Differential Revision: D5189667
fbshipit-source-id: 4818e99e2c2c90788e70e0b8b6204ec6f471d37d
Summary: ExpandDims is a trivial utility op which should not be triggering a warning when used by ModelHelper.
Reviewed By: akyrola
Differential Revision: D5117985
fbshipit-source-id: 5589f46f58458f5019924b48602db088563f2fee
Summary:
Make it easier for users by returning from ExtractPredictorNet the list of blobs that must be saved/exported to run a predictor net. Added a test for ExtractPredictorNet
Codemod.
Reviewed By: asaadaldien
Differential Revision: D5176097
fbshipit-source-id: b1af42132459487b8d94fcdde0e4c514da608243
Summary:
This diff is introducing abstractions for parameter sharing for all the
parameters, that are created through new create_param syntax.
Possible use-cases of this parameters sharing:
1. Share params within RNN interface.
2. Some complicated models that might share some of the branches.
3. TODO (next diff): Cross-model parameter sharing.
Reviewed By: salexspb
Differential Revision: D5160935
fbshipit-source-id: c6d40a5ed7ead240cd7db0eb69de6dc5f505b05a
Summary:
This diff is the first step in the effort for refactoring all parameters. As a first step - I'm merging concept of params and computed_params, that is going
to be based on tags instead (in the first version it's still using old data structs to store all the BlobReferences).
Renaming computed_params to non-trainable/non-backprop params should be done is some other diff.
Reviewed By: salexspb
Differential Revision: D5171159
fbshipit-source-id: 68031ca779f053fb266a7c4a2e5b482a3bd9c832
Summary:
Recent diff introduced a duplicate parameter to the model, which would hurt the performance and also affect correctness (duplicate momentum updates, for example). We unfortunately had no checks for duplicate params, outside of data_parallel_model, which fortunately brought this into our attention.
But it is better to have a Validate() function in model_helper, and call that before adding gradient ops and querying for parameters. Added to brew_test calls as well.
Reviewed By: kennyhorror
Differential Revision: D5163458
fbshipit-source-id: 35692e8bfcc359d4e8bc73e6f2358659f6e45ceb
Summary:
This diff is the first step in the effort for refactoring all paramters. As a
first step - I'm merging concept of params and computed_params, that is going
to be based on tags instead (in the first version it's still using old data
structs to store all the BlobReferences).
Renaming computed_params to non-trainable/non-backprop params should be done is
some other diff.
Reviewed By: salexspb
Differential Revision: D5119830
fbshipit-source-id: 2001090a37346eb12abbb234e13e727c288eb8a7
Summary: Add information about the offending param when assertion fires.
Reviewed By: kennyhorror
Differential Revision: D5153625
fbshipit-source-id: 9f5a02bf64ccbdef9d93d346f79e589dfe3ec5be
Summary:
This is going to unblock Nvidia in their work on adding fp16
support to Caffe2. I discussed this with kennyhorror before to make
sure this fits into his work on parameter sharing.
Reviewed By: kennyhorror
Differential Revision: D5127797
fbshipit-source-id: 4db155d320b1862570c23b77c4252bdacbf2296f
Summary:
It looks like AddOperator was never really used (searched across the whole
code-base). In addition to this all model_helper functionality is getting
replaced with Brew, so there I'd prefer to remove this method to reduce the
amount of code touching model.params.
Reviewed By: rayleichen
Differential Revision: D5110425
fbshipit-source-id: f2a88e4c1ce5149d27e809e03da9a86c0867bc4d
Summary: based on our discussion, we want an arg_map in ModelHelper and create arg_scope for that model within brew. Now it is realized
Reviewed By: salexspb
Differential Revision: D5042983
fbshipit-source-id: ddd2c7e9bca1be2f08a32f7252b44d3b60a57996
Summary: It is good practice to provide __dir__ whenever __getattr__ is defined so that tooling will work intelligently. In particular, it is hard to explore the available methods in iPython without tab completion.
Reviewed By: dzhulgakov
Differential Revision: D5006545
fbshipit-source-id: 1a150d91d54637d80b292764513943ff70d971b4
Summary:
The _param_init_net does not exist. All the other places reference
param_init_net instead. So far no one has encountered any problem
because all the passed params are BlobReferences. This diff makes
this assumption explicit.
Reviewed By: azzolini
Differential Revision: D4922930
fbshipit-source-id: e6dbd7a29ea640b7e62fcfec7ced3cc7d149f872
Summary:
rename model_helpers to brew. This is a big diff now. I did these things:
1. replace model_helpers with brew:
find . -type f -exec sed -i 's/model_helpers/brew/g' {} +
2. rename model_helpers.py and model_helpers_test.py
3. rename ModelHelpersTest to BrewTest
4. lowercase all the helper functions to distinguish them from single op
5. run my unittests
6. run converge tests
Reviewed By: salexspb
Differential Revision: D4930465
fbshipit-source-id: f420a1b03238df1cbe9f4426e0b9c43a12119661
Summary:
rename ModelHelperBase to Model.
This is the result of running:
find . -type f -exec sed -i 's/ModelHelperBase/ModelHelper/g' {} +
We had 19 results when fbgs ModelHelperBase. Here is 20 instances because I added 1 test in model_helpers_test.py
Reviewed By: salexspb
Differential Revision: D4928337
fbshipit-source-id: bc4c12b60b90c167e717de50ea9fe17521e142e3
Summary:
This is not a super-elegant, but a working solution to fix Newsfeed-teams problem of extracting a predictor net of a net that has a "side chain" that they want to cut from the middle.
This adds a argument to ExtractPredictorModel that allows one to define "disabled inputs". These are inputs that we want to switch off, so that all operators that depend on that input will be removed from the model.
Differential Revision: D4839953
fbshipit-source-id: 5d16df6f0ec4aac6670e6917efc77abde5d75c95
Summary: To prevent others making the same mistake as I did, check that no op has is_test=0 argument when ExtractPredictorNet is called.
Reviewed By: viswanathgs
Differential Revision: D4796425
fbshipit-source-id: 38c14df6bcc767ec2e6a6e35ee79596a5dab531c
Summary: Adding synchronous optimization on GPUs to the translation training pipeline, via data_parallel_model.Parallelize_GPU, which needs to be updated so there is some way of performing sparse parameter updates (e.g., on embedding tables), whether on GPU or CPU.
Reviewed By: urikz
Differential Revision: D4631914
fbshipit-source-id: 9cdd655f7dbda3f9b2733d459228b3e097892441
Summary:
First, this diff includes a full test of data-parallel LSTM, which confirms it works correctly. To make it work, some changes had to be made:
- cell net/step net external inputs must be namespace scoped
- prevent double-namescoping of cellnet inputs
- make data parallel model understand recurrentnets so the device-mapping works
Reviewed By: salexspb
Differential Revision: D4708840
fbshipit-source-id: 4b0ddc43642d449076a2b6f67ad1c47f84138ff4
Summary:
It has been a pain to save predictor-compatible models from Caffe2. This diff adds function ExtractPredictorNet that takes a training model and outputs a predictor model by removing all operators that are not relevant for prediction, such as backward pass and dequeue-ops for input loading (as in predictor, the input data is external input).
We can also consider including this directly in the predictor exporter for FB usage.
Reviewed By: rpenggithub
Differential Revision: D4693264
fbshipit-source-id: e81abbbec0bd4d717159cf36488d0baaf0130090
Summary: as title. Add num of examples limit for group collect. Add option for enabling sum loss in BatchLRLoss
Reviewed By: xianjiec
Differential Revision: D4602311
fbshipit-source-id: 5b2a244f1f0e9f1ab0f4590e94828fd18d018d8d
Summary:
One can find a reason, why I need gradient for CopyOp in this post - https://fb.facebook.com/groups/1405155842844877/permalink/1639683782725414/
Gradient for CopyOp is trivial in case the device was the same (cpu, or same gpu), but get's a little harder, when the copy was made across two different gpu.
I introduce new operator CopyOnDeviceLike, which has additional second input. The op copies the first input to the same device as the second one. The default implementation is exactly the same as CopyOp, but I specialize it for CUDAContext.
Please, let me know if I'm doing anything wrong here! That's my first caffe2 diff, related to operators definitions.
Reviewed By: Yangqing
Differential Revision: D4557258
fbshipit-source-id: 9494be589cc1e5696bbbfe25b7622aaa4c9efe4a
Summary:
(Ignore the convolution-op related changes, they will be later patched separately)
This diff ignores work from latest few weeks:
- some refactoring of the flow ops
- no_bias setting
- MAP computation (instead of accuracy) for OC
- adaptive learning rate for Xray concepts
- various small bug fixes
Reviewed By: viswanathgs
Differential Revision: D4329500
fbshipit-source-id: 000d4fd22ec408af5290480c788eb86546bff52e
Summary:
Rewrite D3993337 based on new stack.
Comparing to the old one, we need more readers to achieve the same speed. But so far the speed is the same and the new bottleneck is the write bandwidth of trainer. Model quality is the same as the base.
Reviewed By: azzolini
Differential Revision: D4310803
fbshipit-source-id: 6d04ae8040c1ee7caa9aea5287f054e73fbe325a
Summary:
This is a first step in improving our RNN story. It provides a wrapper around current RecurrentNetworkOp implementation which infers most of the redundant parameters and makes API much simpler.
Also in order to support general step nets I added an extra argument to the RecurrentNetworkOp.
Future work:
1. Inferring step net output and internal blobs (scratches) sizes and type
2. Avoid accessing blobs by names in c++ part
3. Remove requirement for inputs / output 1:1 correspondence in the step net
4. Make python API support networks with operators like Sum being on the boarder of the Cell net (currently there is an issue with such networks where gradient blobs which are on the side are not explicitly created).
Differential Revision: D4268503
fbshipit-source-id: f8a66491c2b55daa730caeed7e9f2b3921541b49
Summary:
This renames the "Snapshot" op name to "Checkpoint" as we discussed earlier.
The early Snapshot name is still available, but we should move to the new name and
eventually deprecate the old name.
The Python SnapshotManager should be also changed, cc azzolini
Reviewed By: dzhulgakov
Differential Revision: D4272021
fbshipit-source-id: 4b8e029354416530dfbf0d538bfc91a0f61e0296
