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

Add support for Upsample op in CAFFE #625

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gasgallo
Copy link
Contributor

Currently it's possible to simulate UpSample op in CAFFE using a deconvolution layer, but this supports only a bilinear kind of resizing of the image. Instead we can implement a real UpSample op with nearest neighbor algorithm and exploit the fact that nearest neighbor algorithm is already implemented in MACE (but not properly wired).

Using resize_nearest_neighbor instead of deconvolution improves image upsampling speed by 2-4x in my tests.

Add resize nearest neighbor output shape computation

Update dockerfile and caffe patch

Update resize_nearest_neighbor implementation and conversion

Fix bugs and update GPU implementation

Align resize_nearest_neighbor with resize_bilinear code

Use scale value

Fix typo

Align kernel compute arguments

Remove comments

Align caffe_converter

Remove unused code
mkdir build && cd build && \
cd python && for req in $(cat requirements.txt) pydot; do pip install $req; done && cd ..

COPY upsample.patch .
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry for the late review.
Thanks for your wonderful code, but as you know, we can not modify the caffe code and maintain a nonstandard version, perhaps we can make this pull request stay here and help the others, but it can not be merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about keeping the original Dockerfile as it was and create a Dockerfile-upsample that contains the patched version?

Copy link
Collaborator

@lu229 lu229 Apr 21, 2020

Choose a reason for hiding this comment

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

@gasgallo I read the code and had a question, how about developping a ResizeNearestNeighbor op in caffe like in tensorflow? or developping a UpSample op like in ONNX, in that cases, there are no need to change the mace's code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lu229 can you elaborate? I'm not sure I understand what you mean

Copy link
Collaborator

@lu229 lu229 Apr 21, 2020

Choose a reason for hiding this comment

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

@gasgallo MACE support the UpSample operator for ONNX, and support ResizeNearestNeighbor operator for Tensorflow, What I means is that, If you simulated UpSample op in CAFFE like in the ONNX, or simulated ResizeNearestNeighbor op in CAFFE likein the Tensorflow, there is no need to modify MACE's c++ code(only need to modify the python code for convert). I think perhaps this is a better selection?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@gasgallo Perhaps I did not understand your problem, why dose you need calling shape_inference before convert_ops? The shape inferences in ONNX and caffe and tensorflow are different, I think if you add the op in caffe, perhaps you need add it as the same the other caffe operators?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lu229 because current caffe upsample layer is like onnx one, that provides a scale factor, therefore we need to know upsample input size to calculate its output size that is required by MACE resizenearestneighbor op.

The other option is that caffe upsample layer directly provides output shape, like happens in tf.image.resize_nearest_neighbor.

I was wondering if the first solution would work? If not, I will consider the second option.

Copy link
Collaborator

@lu229 lu229 Apr 25, 2020

Choose a reason for hiding this comment

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

@gasgallo Perhaps I still did not understand your problem, but I can try to reply. Yes you can refer to the infer_shape_conv_pool_shape function in tools/python/transform/shape_inference.py, which is used to infer the output shape of conv operator, when you infer the Upsample's output shape, you can get the input shape by the _output_shape_cache variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lu229 sorry for confusion, I'll write more in detail right now.

My CAFFE upsample op is defined in a similar way as ONNX upsample op, that takes a scale parameter, so we don't know output shape a priori. For MACE we need to know output shape when calling self.add_tensor.

This works in onnx_converter.py because we can use self._graph_shapes_dict object that contains all pre-computed shapes, and it is created in self.extract_shape_info before calling self.convert_ops.

This is convert_upsample in onnx_converter.py:

    def convert_upsample(self, node):
        op = self.convert_general_op(node)
        del op.input[1:]  # cut all unnecessary inputs (onnx>=1.5)

        output_size = self._graph_shapes_dict[op.output[0]]
        output_size = np.array(output_size[-2:]).astype(np.int32)
        if node.attrs['mode'] == 'nearest':
            op.type = MaceOp.ResizeNearestNeighbor.name
            size_tensor_name = op.name + ":size"
            self.add_tensor(size_tensor_name, output_size.shape,
                            mace_pb2.DT_INT32, output_size)
            op.input.append(size_tensor_name)
        else:
            op.type = MaceOp.ResizeBilinear.name
            size_arg = op.arg.add()
            size_arg.name = MaceKeyword.mace_resize_size_str
            size_arg.ints.extend(output_size.tolist())

        align_corners_arg = op.arg.add()
        align_corners_arg.name = MaceKeyword.mace_align_corners_str
        align_corners_arg.i = node.attrs.get('align_corners', 0)

And this is run() in onnx_converter.py:

    def run(self):
        graph_def = self._onnx_model.graph
        self.extract_shape_info(graph_def)
        self.convert_tensors(graph_def)
        self.convert_ops(graph_def)
        return self._mace_net_def

In caffe_converter.py instead, shape_inferer.run() is called after self.convert_ops, therefore all output shapes are not available during ops conversion, so I cannot compute output shape of upsample op.

This is run() in caffe_converter.py:

    def run(self):
        self.convert_ops()
        shape_inferer = shape_inference.ShapeInference(
            self._mace_net_def,
            self._option.input_nodes.values())
        shape_inferer.run()
        self.replace_output_tensor_name()
        return self._mace_net_def

And shape_inferer.run() cannot be called before self.convert_ops because it requires self._mace_net_def to be already completely defined (that happens in self.convert_ops).

@lu229 Let me know if this makes my point clear.

Copy link
Collaborator

@lu229 lu229 Apr 26, 2020

Choose a reason for hiding this comment

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

@gasgallo Thanks for your detailed description, now I understand your problem, yes you need to define the upsample as same as the other Caffe operators, for the shape inference in Caffe and ONNX is different.

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

Successfully merging this pull request may close these issues.

None yet

2 participants