Generalize IO API to support any number of data / labels

[pluskid]

• Modify MXDataIter interface
• Modify Model train to use extended API
• Modify MNIST example
• Modify Model predict to use extended API
• Modify array iterator
• Lint and clean up
• Fix unit-test

[piiswrong]

You might want to merge in my PR here:#456

[pluskid]

@piiswrong Yes, please merge that PR and I will rebase later.

[piiswrong]

Do we need to differentiate between data and label?

[pluskid]

The only difference is that during training data and label is copied into the network. But during prediction only data is copied. During evaluating on the validation set, data and label are copied into different places. I also found it a bit redundant but I did not have a better way to handle this in more general case.

[piiswrong]

I see.
What's the change to interface?
Could you summarize it and put it somewhere to make it easier for people working with old interface?

[pluskid]

@piiswrong The main change is:

• Originally, we use postfix _data and _label to detect which is data and which is label, and only support one data, one label for input.
• Now there is no special naming convention. The user specify the names of the data in the data iterator, and those names should match the corresponding names (by default data and softmax_label, to keep maximum backward compatibility) in the symbol. And multiple data/label could be used if the data iterator provides.

I will make a more detailed document soon.

[pluskid]

@tqchen Hmm, I did not notice that. I agree this is a potential problem with MXDataIter. I'm not familiar with the underlying data loading and prefetching implementation. But I think there might be a simple way (not making the code too messy) to wire with the Python iterator API so that the first mini-batch could be fetched to check the data shape and batch size without doing an explicit reset.

[tqchen]

Yes, that will need some special treatment in python API, i.e. have a cached instance that will be returned in the call to the next, instead of calling MXIterNext, and support a function like peek to get the instance without advancing the iterator(in impl put that into cached inst)

[piiswrong]

Thanks for the summary!

With regard to shape check, probably implement a peek method?

[tqchen]

I guess the peek method from python API might be a good idea, may need to implement peek, next on base-class, and ask the child class to implement a special _next method to get the data.

[tqchen]

I think the current code is ready to be merged after we get the peek

[pluskid]

k, I should be able to figure that out by the weekend

[pluskid]

Actually I would like to propose to simplify of the current data iter base class. I think the idea is to have as less methods as possible so that the user should be able to write a customized data iter very easily, say by just writing a for loop with Python generator api. Common tools like shuffling within a large cached in-memory pre-loaded mini batches could be provided so that the users do not need to implement the same thing over and over again. But this requires more efforts and discussion about the interface.

[tqchen]

As long as we have a fixed set of api, it is OK. For example, having a subclass Iter that can take a generator, while have default impl for all the other functions

[pluskid]

@tqchen Yes, but writing default impl, for example, for iter_next, or getpad on the iterator object is not easy, if we assume all we know is a generator from the end user. My understanding is that those functions are there to help it easier to implement the wrapper MXDataIter (or maybe even array data iter). But there is no need to require all data iterators to implement those method because:

• Those methods (e.g. next_iter, getdata, etc.) are never explicitly called, since all the usage of data iterator is for batch in data_iter: ... -- therefore, the only required interface is a python iterator interface, the user could implement it explicitly by writing the __iter__ and next function, or by just wrapping a generator. But I might be wrong. If there is other potential usage of data iterator that requires a richer interface, please let me know.
• I think it is not true that under different implementation paradigm it is equally easy to implement all the interface functions. For example, if I simply provide a generator to iterate over all the data. In order to implement iter_next, I will need to call the underlying generator, cache the results, and catch the end-iter exception, etc. Similarly, in order to implement getdata on the iterator object, I will need to explicitly fetch data from the underlying iterator, and store it somewhere.

[tqchen]

I get your point, yes I agree, iter and next are two things we need. The rest interface that are helpful(but not needed) is the getpad(to get number of padded instance, for predictor)

• getpad, which was used for reporting number of padded data when data, but as you did in the update, it can readily be incorporated into the databatch

We might need the provide_data and provide_label, either as a list of functions. In that sense, I agree that the peek should be implemented in the subclass, to support provide_data and label for MXDataIter

[pluskid]

Yes getpad is not being suggested to be removed, they now exists in the batch object, which is the object returned in each iteration that contains the data, label and other information (including the pad) for that minibatch.

[tqchen]

Sounds good to me

[piiswrong]

Since we now support multiple data/label, the metrics interface probably should also be updated?

[pluskid]

Yes that is one aspect that we also need to consider a bit. Currently I just replaced all existing metrics with one that naively deal with pred and label list of the same length, by evaluating them one by one and then accumulate. This should be backward compatible with existing behavior, but for long term, we need to think about what would be best interface.

[tqchen]

Yes, I guess that should be addressed in a separate issue, let us first aim to get this merged

[pluskid]

Please review and merge it. I added a simple cache for the first batch to avoid calling reset. The discussed simplification of base iter interface could wait later, otherwise accumulating conflicts with base could get quite complicated. I hope I did not overwrite something during merging.

[tqchen]

This logic is recently added by mu for distributed training, so maybe needed here. The logic is to run epoch_size batches on each machine, to avoid different number of epochs on each machine

[tqchen]

I have one comment on support epoch_size parameter which is added by @mli for distributed training. Maybe need to add that back.

This is used when different machines have different batches on their local data, and we need to make sure each machine run the same number of batches in one epoch in distributed setting. Otherwise one machine will run additional batch, and hang because other machine did not send statistics over in BSP setting

[tqchen]

Other parts LGTM

[pluskid]

I tried to recover that part here.

[tqchen]

cool, this is merged