Allow arbitrary output for HDF5 layer

[pluskid]

modified HDF5 layer to allow arbitrary output of the layer (not only label and data any more). This is useful for example, when there are multiple stream of data, say one part of the data needs to go through convolution while the other part of the data should go directly to higher fully connected layer. In this case, people could split the dataset and store them as different "dataset" in the HDF5 file.

Test case is also modified to cover the new behavior. The modification is backward compatible (except that now we are not constraining the dimension of the label dataset).

[jeffdonahue]

This is great -- I've wanted it for a while; thanks @pluskid!

Tying the HDF5 dataset names to the top blob names seems like a natural way to do it, but it will break models for users that assume the HDF5 dataset names will always be "data" and "label" and then name their top blobs something else. Perhaps if taking the blob name to be the HDF5 dataset name results in a failed load, we should fall back to the existing two names? Or we could just accept the breaking change in this case.

[jeffdonahue]

instead of removing this line, change it to:

virtual inline int MinTopBlobs() const { return 1; }

this will require the layer to have at least 1 output

[pluskid]

@jeffdonahue I am OK to add a fallback to data and label but personally I think this makes the code a bit messy -- what if we have three dataset to load? What if there was just a typo in user's model file?

And I think since HDF5 layer is a bit new and there is no detailed document yet on the old behavior, it won't be too difficult for users to adopt the new behavior. Actually when I started using HDF5 layer, I simply followed the ipython notebook example to name the two dataset data and label, and I didn't know the two names are hard-coded until I read the caffe code.

[jeffdonahue]

@jeffdonahue I am OK to add a fallback to data and label but personally I think this makes the code a bit messy -- what if we have three dataset to load? What if there was just a typo in user's model file?

I agree the code would be a little messy. I was thinking the fallback would only happen if the user had exactly 2 top blobs -- i.e., their hdf5data layer specification would work using the current Caffe code (otherwise it would fail as usual). If it failed just due to a typo in the user's model file, the fallback presumably wouldn't succeed either (or at least it's hard for me to come up with a scenario when the fallback would succeed but should have failed).

Anyway, I think I'm personally fine with breaking the current workflow since this approach is cleaner and, as you said, it's kind of messy to fix and the current workflow is basically undocumented.

[pluskid]

Hi @jeffdonahue, thank you for the code review! I fixed them. Sorry my local cluster is temporarily broken so I'm pushing the code directly to see the Travis CI build output.

[jeffdonahue]

remove these two lines -- unused now (same in the CPU version if they are there as well)

[jeffdonahue]

Thanks for the fixes -- see my additional comment above. Once that's fixed, if we can hear from @shelhamer, @longjon, @sergeyk, or one of the other core devs that it's ok to break how the current HDF5 data layer works, I should be able to merge this. (Or if not ok, will need to add the fallback before merging.)

[sergeyk]

Fine to break current implicit name expectation, provided the example still works.

[shelhamer]

Agreed.

On Mon, Sep 29, 2014 at 3:33 PM, Sergey Karayev notifications@github.com
wrote:

Fine to break current implicit name expectation, provided the example
still works.

—
Reply to this email directly or view it on GitHub
#1183 (comment).

[pluskid]

all fixed now!

[jeffdonahue]

Whoops, this PR should have been made to dev. I rebased this on dev and merged it (24c9d8a). Thanks again @pluskid!