Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add memory data layer to pass data directly into the network #196

Closed
wants to merge 6 commits into from
Closed

Add memory data layer to pass data directly into the network #196

wants to merge 6 commits into from

Conversation

kloudkl
Copy link
Contributor

@kloudkl kloudkl commented Mar 9, 2014

As stated in the big umbrella issue #148, current version of DataLayer is bound to specific data source and data format. Ideally data from any external source and in any format should be able to stream though the network. This PR provides an in memory gateway for such a purpose.

Although the memory data layer can be straightforwardly integrated with #120 to enable feeding raw images in memory into the network, it simply copies memory blobs at the moment. If anyone is interested in such a design, I will further refine and extend it.

@shelhamer
Copy link
Member

Passing input in memory is conveniently general purpose, and I could see its use for camera input or even realtime operation.

A memory input path should be helpful in combination with data processing layers by #148 and batch size adapting in #195. If we adopt this data layer, should we not drop the input field format and rewrite the examples with this new layer? To merge it this should work as simply as the input fields.

The only question is whether the current input field method suffices? Dimensions are still programmable in-memory by modifying the ProtoBuf definition once it is loaded. An example of this is in the ImageNet deployment model imagenet.prototxt.

@kloudkl
Copy link
Contributor Author

kloudkl commented Mar 11, 2014

Realtime operations usually use multi-threading and therefore need a thread safe data buffer behind the scene. The Intel thread building block has the suitable concurrent containers but it is a heavy weight dependency.

The input field is not very straightforward to use. The different look of the deployment prototxts make the users wonder why don't they have a data layer like the train and test model definitions. But it should be kept for backward compatibility. It is up to the users to choose which method to use.

@shelhamer
Copy link
Member

Agreed about the confusion over the input fields. Having data layers throughout for every phase and purpose will be clearer. Please update the example model definitions accordingly (imagenet.prototxt, lenet.prototxt, etc.). I'll look forward to merging, if this is done in such a way that it is simpler than input fields.

Realtime and its input needs naturally can wait for its own PR.

@shelhamer
Copy link
Member

@sguada I believe you had review comments for this PR? @kloudkl, looking forward to merge post-review!

template <typename Dtype>
void MemoryDataLayer<Dtype>::SetUp(const vector<Blob<Dtype>*>& bottom,
vector<Blob<Dtype>*>* top) {
CHECK_EQ(bottom.size(), num_data_blobs_) <<
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kloudkl The idea behind of Data Layers is that they don't any bottom blobs, they just provide top blobs of data for the next layers.
Your current implementation only copy a bottom blob into the top blob, but which layers defines the bottom blob? which part of the code will be in charge of copying the data in there?

@sguada
Copy link
Contributor

sguada commented Mar 14, 2014

@kloudkl I think having a memory data layer is a good idea, but the current design and implementation is flawed.
Also the idea of a memory data layer, should be in agreement with current python and matlab wrappers.
A full test of a network containing a memory data layer will be needed.

@shelhamer
Copy link
Member

Seconding @sguada, it will be nice to replace the potentially confusing "input" fields with a memory data layer, but this layer must keep compatibility with the wrappers / update the wrappers to use the new layer.

@Yangqing
Copy link
Member

In general this seems to be coercing data preprocessing into a layer, which I do not quite support - the memory data layer expects a blob anyway, thus not reducing complexity at all. I think a better way is to write utility functions under util/ that enables different data preprocessing, such as OpenCVImageToBlob() and RawMemoryChunkToBlob().

@sguada
Copy link
Contributor

sguada commented Mar 14, 2014

@Yangqing I also think data preprocessing should be separated from the memory data layer, it could be a new type of layer, or some part of all data layers. I think it will be cleaner to be a separated layer, but it will require some extra memory.

@Yangqing
Copy link
Member

@sguada I agree. In my mind a layer does two things:

(1) takes in a set of blobs, and spits a set of other blobs (or no blobs)
(2) takes no blobs, and produces blobs from e.g. disks, where the source is defined in its layer param.

A memory data layer, as I expect it, is to load a dataset in memory (the dataset will still be specified as some sort of on-disk file), and then spit blobs directly from memory. In this sense it will be not much different from datalayer, the only difference is where the data come from (our other working direction of separating dataread and datapreprocess fits in this purpose nicely).

I feel that having the input fields is fine - it tells us what data size the net expects, and needs to be specified somewhere anyway. By putting it at the top of the protobuf it makes it easier to examine.

@shelhamer
Copy link
Member

@Yangqing, I think an input data layer that only provides a top but exposes
assignment to its top blobs, as suggested by @sguada, would be nice. It's
only the input fields (with declared dimensions and all) under another
name, but it would

On Fri, Mar 14, 2014 at 4:51 PM, Yangqing Jia notifications@github.comwrote:

@sguada https://github.com/sguada I agree. In my mind a layer does two
things:

(1) takes in a set of blobs, and spits a set of other blobs (or no blobs)
(2) takes no blobs, and produces blobs from e.g. disks, where the source
is defined in its layer param.

A memory data layer, as I expect it, is to load a dataset in memory (the
dataset will still be specified as some sort of on-disk file), and then
spit blobs directly from memory. In this sense it will be not much
different from datalayer, the only difference is where the data come from
(our other working direction of separating dataread and datapreprocess fits
in this purpose nicely).

I feel that having the input fields is fine - it tells us what data size
the net expects, and needs to be specified somewhere anyway. By putting it
at the top of the protobuf it makes it easier to examine.


Reply to this email directly or view it on GitHubhttps://github.com//pull/196#issuecomment-37708581
.

@kloudkl
Copy link
Contributor Author

kloudkl commented Mar 15, 2014

This is exactly an attempt to implement the simplest possible data layer consistent with #148 whose discussion is hard to push forward without a concrete example. Yes, the intention is to separate reusable data io, format handling and preprocessing from the data layers.

The input blobs come from some types of DataSource which does not necessarily have to be a layer. And there also need to be DataSink for the output blobs (#213).

The updated example protos show that the data layers can also specify the data sizes that they expect.

It is tempting to directly assign data to the top blobs which are not constant. But then you take the risk of conflicts caused by multiple writing threads or processes.

@kloudkl
Copy link
Contributor Author

kloudkl commented Mar 23, 2014

Closing this which is continued by #251.

@kloudkl kloudkl closed this Mar 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants