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

Generalize bilinear filler to for N-D multilinear filler #3984

Closed

Conversation

christianpayer
Copy link

This branch changes the bilinear filler (#2213) to support an arbitrary number of dimensions. Also, different scaling factors for each dimension are supported. I changed the name of the filler from "bilinear" to "linear", as it suits better for its multi-dimensional functionality.

The usage of this filler is the same as before (see comments, or #2213). Tests are not included, but I plan to write some using python scikit-image.

@shelhamer
Copy link
Member

While there is no reason this filler should be limited to 4-D, as it is now, this breaks backward compatibility as-is since there are existing definitions that make use of bilinear. See our upgrade proto code for an idea of how to automatically upgrade old definitions -- this is our practice for schema changes.

See #2213 for some earlier discussion of the naming. linear doesn't feel quite right since that could be nearly anything, although bilinear is admittedly over specific. Perhaps interp would do? The combination of a linear layer like Convolution and an interp filler might be clear enough.

Thoughts @longjon?

@longjon
Copy link
Contributor

longjon commented Apr 12, 2016

In the usual interpolation sense (i.e., as a function of coordinates), linear is not correct; bilinear functions are nonlinear functions which are separately linear in each of their arguments. interp is too generic, as there are non-bilinear interpolation methods (e.g., biquadratic or bicubic) which can be implemented using fillers and convolutions.

The generalization of bilinear is multilinear. Perhaps these could be accepted as synonyms.

@christianpayer
Copy link
Author

Thanks for your suggestions. I agree that linear is not the best name and that multilinear is much better.
If you agree to use multilinear I will change it and adapt the code for upgrade proto.

: Filler<Dtype>(param) {}
virtual void Fill(Blob<Dtype>* blob) {
CHECK_EQ(blob->num_axes(), 4) << "Blob must be 4 dim.";
CHECK_EQ(blob->width(), blob->height()) << "Filter must be square";
CHECK_GT(blob->num_axes(), 3) << "Blob must have at least 3 dimensions.";

Choose a reason for hiding this comment

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

Shouldn't this be CHECK_GE, if you want to test the condition "at least 3"?

Copy link
Author

Choose a reason for hiding this comment

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

You are right, thanks for pointing that out. I will change it. I never tested the code for 1d inputs, but it should work, too.

@shelhamer
Copy link
Member

@christianpayer I second multilinear, so if you agree then go ahead with the new name.
Once done, please squash your commits into a new commit with a message that reflects the new filler (in particular the new name and generality of dimensions). Thanks!

@shelhamer shelhamer changed the title add support for 3D linear upsampling Generalize bilinear filler to for N-D multilinear filler Apr 14, 2016
@christianpayer
Copy link
Author

@shelhamer Thanks for the comments. I changed the name to multilinear, squashed the commit and changed the commit message.
I did not edit upgrade_proto.cpp, as I am not sure, where/how to change the code there. As far as I can see an upgrade of a filler name has not been done before. Do you want me to implement something like

bool NetNeedsWeightFillerUpgrade(const NetParameter& net_param);
void UpgradeNetWeightFiller(NetParameter* net_param);

in upgrade_proto.hpp/cpp?

@shelhamer
Copy link
Member

@christianpayer adding the check NetNeedsWeightFIllerUpgrade() and update UpgradeNetWeightFiller() is the idea. You'll need to be a bit familiar with the C++ interface to protobuf, but hopefully the existing methods can be a guide. Here's a quick sketch to illustrate key pieces:

for (int i = 0; i < net_param->layer_size(); ++i) {
      if (net_param->layer(i).has_convolution_param()) {
          FillerParameter* filler_param = net_param->mutable_layer(i)->mutable_convolution_param()->mutable_filler_parameter();
          if (filler_param.type() == "bilinear") {
              filler_param->set_type("multilinear");
          }
      }
}

Care to take a stab at it and we'll go from there?

Christian Payer added 2 commits April 18, 2016 12:32
rename weight filler "bilinear" to "multilinear"
and generalize it for N dimensions with variable
scaling factors per dimension
@christianpayer
Copy link
Author

@shelhamer Thanks for the code snippet. I just implemented upgrade_proto for weight fillers and hope that this is what you had in mind.

@christianpayer
Copy link
Author

Please do not merge this PR, as I implemented in the meantime a more general interpolation filler, that allows linear/cubic/lanczos interpolation and rewrites most of the code of this PR.
Next week I will create a new PR with this more general filler.

@christianpayer
Copy link
Author

Closed in favour of #4198.

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