-
Notifications
You must be signed in to change notification settings - Fork 18.6k
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
Hdf5 output layer #252
Hdf5 output layer #252
Conversation
Looks great! One thing, could you make sure that the Forward/Backward GPU mode functions are split into separe |
@@ -142,4 +142,30 @@ void hdf5_load_nd_dataset_helper( | |||
file_id, dataset_name_, blob->mutable_cpu_data()); | |||
} | |||
|
|||
template <> | |||
void hdf5_save_nd_dataset<float>( | |||
const hid_t file_id, const string dataset_name, const Blob<float>& blob) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kloudkl one probably want to save multiple blobs in the same HDF5 file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation assumes that a dataset corresponds to a blob as hdf5_load_nd_dataset indicates. Future needs would be satisfied in some future PRs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, hdf5_load_nd_dataset can load different Blobs from the same file, but it's true it could be a future feature.
The ImagesLayer.cpp and WindowsDataLayer.cpp need to be split in another PR. |
Agree with @kloudkl on both counts. Is a commit to split to |
This introduces a new layer and proto field so it needs to be coordinated with @jeffdonahue's #219. |
The ImagesLayer.cpp and WindowsDataLayer.cpp are coming up. |
@kloudkl I think ImagesLayer.cpp and WindowsDataLayer.cpp should go in a different PR. Leave this one only for HDF5Output |
Yes, they are in #253. |
@shelhamer let's avoid blocking situations as agreed. let's merge this, then 219, then fix whatever needs fixin' |
@sergeyk agreed–just flagging for Jeff so it isn't missed. |
The changes to src/caffe/proto/caffe.proto are inline with #219.
But I am afraid that adding |
Thanks @kloudkl |
Hdf5 output layer
Add support for GPU transforms to reduce CPU load
This PR resolves #213 and also closes #220.
@sergeyk, please review and check whether this is what you asked for.