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

Conflicting Parameters in WindowDataLayer #1125

Closed
ronghanghu opened this issue Sep 21, 2014 · 6 comments
Closed

Conflicting Parameters in WindowDataLayer #1125

ronghanghu opened this issue Sep 21, 2014 · 6 comments

Comments

@ronghanghu
Copy link
Member

In #995, the data layers are re-arranged into a hierarchical structure. After this modification, the WindowDataLayer has conflicting parameters in TransformationParameter and in WindowDataParameter:

message TransformationParameter {
  // For data pre-processing, we can do simple scaling and subtracting the
  // data mean, if provided. Note that the mean subtraction is always carried
  // out before scaling.
  optional float scale = 1 [default = 1];
  // Specify if we want to randomly mirror data.
  optional bool mirror = 2 [default = false];
  // Specify if we would like to randomly crop an image.
  optional uint32 crop_size = 3 [default = 0];
  optional string mean_file = 4;
}

and

message WindowDataParameter {
  // Specify the data source.
  optional string source = 1;
  // For data pre-processing, we can do simple scaling and subtracting the
  // data mean, if provided. Note that the mean subtraction is always carried
  // out before scaling.
  optional float scale = 2 [default = 1];
  optional string mean_file = 3;
  // Specify the batch size.
  optional uint32 batch_size = 4;
  // Specify if we would like to randomly crop an image.
  optional uint32 crop_size = 5 [default = 0];
  // Specify if we want to randomly mirror data.
  optional bool mirror = 6 [default = false];
  // Foreground (object) overlap threshold
  optional float fg_threshold = 7 [default = 0.5];
  // Background (non-object) overlap threshold
  optional float bg_threshold = 8 [default = 0.5];
  // Fraction of batch that should be foreground objects
  optional float fg_fraction = 9 [default = 0.25];
  // Amount of contextual padding to add around a window
  // (used only by the window_data_layer)
  optional uint32 context_pad = 10 [default = 0];
  // Mode for cropping out a detection window
  // warp: cropped window is warped to a fixed size and aspect ratio
  // square: the tightest square around the window is cropped
  optional string crop_mode = 11 [default = "warp"];
}

Parameters such as scale and mean_file appear twice, which cause serious problems and break down the finetune-pascal example. Currently WindowDataParameter is using mean_file from TransformationParameter, scale, crop_size and mirror from WindowDataParameter. Especially, the mean_file in TransformationParameter is used, while it is specified in WindowDataParameter in finetune-pascal example and in the R. Girshick's R-CNN repo).

I found my experimental result these days so weird and I later found that the WindowDataLayer is not working at all due to this issue. I am going to make a pull request to resolve it.

We should also add unit test for WindowDataLayer.

@sguada
Copy link
Contributor

sguada commented Sep 21, 2014

Could you try #1070 and if it solves the problem?

On Sunday, September 21, 2014, Ronghang Hu notifications@github.com wrote:

In #995 #995, the data layers are
re-arranged into a hierarchical structure. After this modification, the
WindowDataLayer has conflicting parameters in TransformationParameter and
in WindowDataParameter:

message TransformationParameter {
// For data pre-processing, we can do simple scaling and subtracting the
// data mean, if provided. Note that the mean subtraction is always carried
// out before scaling.
optional float scale = 1 [default = 1];
// Specify if we want to randomly mirror data.
optional bool mirror = 2 [default = false];
// Specify if we would like to randomly crop an image.
optional uint32 crop_size = 3 [default = 0];
optional string mean_file = 4;
}

and

message WindowDataParameter {
// Specify the data source.
optional string source = 1;
// For data pre-processing, we can do simple scaling and subtracting the
// data mean, if provided. Note that the mean subtraction is always carried
// out before scaling.
optional float scale = 2 [default = 1];
optional string mean_file = 3;
// Specify the batch size.
optional uint32 batch_size = 4;
// Specify if we would like to randomly crop an image.
optional uint32 crop_size = 5 [default = 0];
// Specify if we want to randomly mirror data.
optional bool mirror = 6 [default = false];
// Foreground (object) overlap threshold
optional float fg_threshold = 7 [default = 0.5];
// Background (non-object) overlap threshold
optional float bg_threshold = 8 [default = 0.5];
// Fraction of batch that should be foreground objects
optional float fg_fraction = 9 [default = 0.25];
// Amount of contextual padding to add around a window
// (used only by the window_data_layer)
optional uint32 context_pad = 10 [default = 0];
// Mode for cropping out a detection window
// warp: cropped window is warped to a fixed size and aspect ratio
// square: the tightest square around the window is cropped
optional string crop_mode = 11 [default = "warp"];
}

Parameters such as scale and mean_file appear twice, which cause serious
problems and break down the finetune-pascal example (the mean_file in
TransformationParameter is used, while it is specified in
WindowDataParameter in finetune-pascal example and in the R-CNN repo.

I found my experimental result these days so weird and I later found that
the WindowDataLayer is not working at all due to this issue. I am going to
make a pull request to resolve it.

We should also add unit test for WindowDataLayer.


Reply to this email directly or view it on GitHub
#1125.

Sergio

@ronghanghu
Copy link
Member Author

@sguada I haven't tried yet, but I really think we should keep a consistent inferface for data layers, and it is changing a bit too often recently. Otherwise, all models in model zoo, and other software like R-CNN may all break down.

@sguada
Copy link
Contributor

sguada commented Sep 21, 2014

In theory this change should be transparent to existing data layers in
models or rcnn.
But I also share your concerns.

On Sunday, September 21, 2014, Ronghang Hu notifications@github.com wrote:

@sguada https://github.com/sguada I haven't tried yet, but I really
think we should keep a consistent inferface for data layers, and it is
changing a bit too often recently. Otherwise, all models in model zoo, and
other software like R-CNN may all break down.


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

Sergio

@shelhamer
Copy link
Member

The point of keeping the old fields was that they are needed for
backwards-compatibility. All the actual code should refer to the
transformation params and not the old layer params because the old params
are automatically / manually upgradeable.

I'll take another look at this too and would appreciate testing. The
duplicated fields are needed for now though for upgradability.

On Sunday, September 21, 2014, Sergio Guadarrama notifications@github.com
wrote:

In theory this change should be transparent to existing data layers in
models or rcnn.
But I also share your concerns.

On Sunday, September 21, 2014, Ronghang Hu <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

@sguada https://github.com/sguada I haven't tried yet, but I really
think we should keep a consistent inferface for data layers, and it is
changing a bit too often recently. Otherwise, all models in model zoo,
and
other software like R-CNN may all break down.


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

Sergio


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

Evan Shelhamer

@shelhamer
Copy link
Member

@ronghanghu I have traced the problem to the data parameter upgrade skipping WindowDataLayer: https://github.com/BVLC/caffe/blob/master/src/caffe/util/upgrade_proto.cpp#L509-L527

Could you check my fix in #1126?

I agree that WindowDataLayer needs tests to protect against issues while the data layer design is improved. If you could PR tests that would be very helpful.

@shelhamer
Copy link
Member

#1126 fixes the parameterization and #1136 adds tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants