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

Breaking changes in model output layers format #587

Open
SIGLOST opened this issue Nov 13, 2024 · 11 comments
Open

Breaking changes in model output layers format #587

SIGLOST opened this issue Nov 13, 2024 · 11 comments

Comments

@SIGLOST
Copy link

SIGLOST commented Nov 13, 2024

Hi Marcos,

Can you please explain the motivation for changing the models output format in this commit?

This breaks the compatibility with all the models previously converted to ONNX (currently in production) that are still using the old format right?

Thank you

@marcoslucianops
Copy link
Owner

marcoslucianops commented Nov 14, 2024

The TensorRT sometimes doesn't keep the output order, making an issue with the parser function. That's why it's using 1 output now instead of 3.

@marcoslucianops
Copy link
Owner

You can change the nvdsparsebbox_Yolo.cpp file (based on the old file) to set 3 outputs again.

@SIGLOST
Copy link
Author

SIGLOST commented Nov 14, 2024

I think that's the same issue identified in this DeepStream forum topic when running your custom parser library with nvinferserver.
They suggested obtaining the output layers by name instead of by order.

If that's the only reason for the change, I kindly ask you to consider preserving the previous format, and adapt nvdsparsebbox_Yolo to read the layers by name.

Your great work in this repository is widely spread (example), and I believe besides me there might be several folks using ONNX models converted to the previous output format. IMHO the impact of this change is huge because it requires running the export scripts again using the original weights files.

Thank you

@marcoslucianops
Copy link
Owner

marcoslucianops commented Nov 14, 2024

For paddle (PP-YOLOE, RT-DETR, etc) models, I can't set easily the output names for the ONNX model, so I can't get the layers by the name.

@SIGLOST
Copy link
Author

SIGLOST commented Nov 14, 2024

For paddle (PP-YOLOE, RT-DETR, etc) models, I can't set easily the output names for the ONNX model, so I can't get the layers by the name.

I think the previous export_ppyoloe.py script can be adjusted to return the layer names explicitly:

class DeepStreamOutput(nn.Layer):
    def __init__(self):
        super().__init__()

    def forward(self, x):
        boxes = x['bbox']
        x['bbox_num'] = x['bbox_num'].transpose([0, 2, 1])
        scores = paddle.max(x['bbox_num'], 2, keepdim=True)
        classes = paddle.cast(paddle.argmax(x['bbox_num'], 2, keepdim=True), dtype='float32')
        
        # Return outputs as a dictionary with named keys
        return {'boxes': boxes, 'scores': scores, 'classes': classes}

@SIGLOST
Copy link
Author

SIGLOST commented Nov 14, 2024

Also here it seems to be missing output_spec

    # Export with named outputs for ONNX
    paddle.onnx.export(
        model, cfg.filename, 
        input_spec=[onnx_input_im], 
        output_spec=['boxes', 'scores', 'classes'],  # Specify named outputs here
        opset_version=FLAGS.opset
    )

@marcoslucianops
Copy link
Owner

return {'boxes': boxes, 'scores': scores, 'classes': classes}
The problem isn't this order, but the order the TRT uses after create the engine.

output_spec=['boxes', 'scores', 'classes']
It doesn't work for paddle.export. That's need to set the shape and it can be different values based on the input size.

@SIGLOST
Copy link
Author

SIGLOST commented Nov 14, 2024

static bool
NvDsInferParseCustomYolo(std::vector<NvDsInferLayerInfo> const& outputLayersInfo, NvDsInferNetworkInfo const& networkInfo,
    NvDsInferParseDetectionParams const& detectionParams, std::vector<NvDsInferParseObjectInfo>& objectList)
{
  if (outputLayersInfo.empty()) {
    std::cerr << "ERROR: Could not find output layer in bbox parsing" << std::endl;
    return false;
  }

  std::vector<NvDsInferParseObjectInfo> objects;

  auto layerFinder = [&outputLayersInfo](const std::string &name) -> const NvDsInferLayerInfo *{
      for (auto &layer : outputLayersInfo) {
        if (layer.dataType == FLOAT &&
            (layer.layerName && name == layer.layerName)) {
          return &layer;
        }
      }
      return nullptr;
  };

  const NvDsInferLayerInfo *boxes = layerFinder("boxes");
  const NvDsInferLayerInfo *scores = layerFinder("scores");
  const NvDsInferLayerInfo *classes = layerFinder("classes");

  const uint outputSize = boxes->inferDims.d[0];

  std::vector<NvDsInferParseObjectInfo> outObjs = decodeTensorYolo((const float*) (boxes->buffer),
      (const float*) (scores->buffer), (const float*) (classes->buffer), outputSize, networkInfo.width, networkInfo.height,
      detectionParams.perClassPreclusterThreshold);

  objects.insert(objects.end(), outObjs.begin(), outObjs.end());

  objectList = objects;

  return true;
}

If we access the output layers by name ^^ we don't need to care about the order the layers are passed (like refereed in the NVIDIA forum, the output tensor order is not even guaranteed to be consistent when using Triton GRPC & nvinferserver) .

This approach is working just fine both for nvinfer & nvinferserver with all the other architectures and conversion scripts.

YoloNAS:
exported_yoloNAS

YoloV8:
exproted_yolov8

Like you said, just for paddle models we get those weird output layers names by default:
exported_ppyoloe

I'm trying to rename them accordingly but the graph keeps getting broken 😭 .

@SIGLOST
Copy link
Author

SIGLOST commented Nov 14, 2024

@marcoslucianops I was able to workaround by using onnx to rename the output layers of the file previously exported with paddle.onnx.export() (please find here the adjusted export_ppyoloe.py script)

ppyoloe_plus_crn_s_80e_coco.onnx summary:
ppyoloe_plus_crn_s_80e_coco_onnx_graph

Deployed it on DeepStream 7.0 with the previous version of DeepStream-Yolo & the change in the parser code to use named output layers:
ppyoloe_plus_crn_s_80e_coco_onnx_triton_pipeline

ppyoloe_plus_crn_s_80e_coco_onnx_webrtc_output

@marcoslucianops
Copy link
Owner

I think the best option is to keep one output layer for the models. The TRT doesn't guarantee the output order when converting from ONNX. To keep the 3 outputs, it's possible to just use the old nvdsparsebbox_Yolo.cpp file.

@Sanelembuli98
Copy link

Can the above be applied to deepstream-app as I am really struggling?

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