-
Notifications
You must be signed in to change notification settings - Fork 18.7k
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
Wrap up SyncedMem resize from @kloudkl; make train/test nets share data blobs #355
Changes from all commits
f2fed7a
f42e347
a278657
1b0c1e0
3b574fd
ff98a33
993c55a
a495223
ccdd521
ccccbdb
fd896bf
7272ca0
5a94f27
7a89d81
43bdc6f
78b35f3
dbba6c2
3011d82
5717ab5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
train_net: "imagenet_train.prototxt" | ||
test_net: "imagenet_val.prototxt" | ||
test_iter: 1000 | ||
test_iter: 200 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed the batch size from 50 to 250, so leaving the setting as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, this is right–I misread this as |
||
test_interval: 1000 | ||
base_lr: 0.01 | ||
lr_policy: "step" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,19 +12,17 @@ namespace caffe { | |
template <typename Dtype> | ||
class Blob { | ||
public: | ||
Blob() | ||
: num_(0), channels_(0), height_(0), width_(0), count_(0), data_(), | ||
diff_() {} | ||
explicit Blob(const int num, const int channels, const int height, | ||
const int width); | ||
void Reshape(const int num, const int channels, const int height, | ||
const int width); | ||
explicit Blob(const int num = 0, const int channels = 0, | ||
const int height = 0, const int width = 0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe remove the default arguments? It would be odd to set some being 0 and some being not 0, but if all are 0, it is effectively just the default constructor Blob(). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kloudkl got rid of the no-arg blob constructor -- the one with the default args sets member variables and does Reshape, so I don't think it's quite the same? |
||
explicit Blob(const Blob& memory_share_blob); | ||
void Reshape(const int num, const int channels, | ||
const int height, const int width); | ||
void ReshapeLike(const Blob& other); | ||
inline int num() const { return num_; } | ||
inline int channels() const { return channels_; } | ||
inline int height() const { return height_; } | ||
inline int width() const { return width_; } | ||
inline int count() const {return count_; } | ||
inline int count() const { return count_; } | ||
inline int offset(const int n, const int c = 0, const int h = 0, | ||
const int w = 0) const { | ||
CHECK_GE(n, 0); | ||
|
@@ -91,8 +89,9 @@ class Blob { | |
int height_; | ||
int width_; | ||
int count_; | ||
size_t space_requirement_; | ||
|
||
DISABLE_COPY_AND_ASSIGN(Blob); | ||
DISABLE_ASSIGN(Blob); | ||
}; // class Blob | ||
|
||
} // namespace caffe | ||
|
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.
What's up with the Makefile changes introduced in 1d4ea4be7e77203306a157580a44f70fb475093e?
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 had added the
+$(OBJ_BUILD_DIR)/%.cuo:
rule (presumably due to the renaming of syncedmem.cpp->syncedmem.cu -- we had no rule for building*.cu
files in the top-levelsrc/caffe
directory), but I moved it down in the list because older versions ofmake
(including the one I use, the default Ubuntu installation) will match the first rule matching instead of the most specific one, so this rule matched*.cu
files in subdirs also, which I fixed by moving it after all other*.cu
rules.The rest of the changes were basically style changes and adding a dependency on the header files
$(HXX_SRCS)
where I happened to notice there wasn't one before (in one case changing from$(PROTO_GEN_HEADER)
, which is a subset of$(HXX_SRCS)
) Sorry for mixing these changes into an unrelated PR...I can remove them from history if desired.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.
Fine to keep it in this PR for convenience, but could you split the Makefile changes into their own commit (or at least mention them in the message for 1d4ea4b)?
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.
done - moved into different commit right before the "use handwritten resize methods" one (github displays it as being the last commit though)