Improve Graph.remove_node, add Graph.remove_subtree - WiP

[Mikhael-Danilov]

• Graph.remove_node will remove dangling outputs
• Graph.remove_subtree method will remove a node and all its dependent elements at once

[ThanatosShinji]

A very interesting idea!

[ThanatosShinji]

I have this situation. I want to remove this Relu node in the red rectangle. According to your code, it will add Relu's next nodes to the removal list. What is supposed to deal with Conv node in the yellow rectangle? As its next node 'Add' is already removed, which makes it useless, keep it or remove it?

[ThanatosShinji]

It's an open discussion. I'm not familiar with the use case of remove_subtree. Maybe you can make the decision, as it's your need.

[Mikhael-Danilov]

It's an open discussion. I'm not familiar with the use case of remove_subtree. Maybe you can make the decision, as it's your need.

Thank you for point. By my plan Conv (and any dangling) node should be removed too, so network graph will be in clean (ready to save & run) state after remove_subtree call. I'll submit fix in a day or two.

[ThanatosShinji]

Actually, you don't need to fix it. The node that creates useless output tensors will be removed by onnx-tool. That's the Conv in the yellow rectangle. After some more model tests, this PR will be merged. Thanks for your contribution!

[ThanatosShinji]

Actually, you don't need to fix it. The node that creates useless output tensors will be removed by onnx-tool. That's the Conv in the yellow rectangle. After some more model tests, this PR will be merged. Thanks for your contribution!

Sorry, my mistake! onnx-tool will add these useless tensors to the graph's output tensor list, which means it won't remove the dangling nodes. Maybe you need to fix it.

[Mikhael-Danilov]

After using onnx-tool some more I came up with separate Graph.remove_dangling_nodes method, which should be called after use on remove_node or remove_subtree.

[ThanatosShinji]

Great! I also think it's better to decouple two functions. As my computer is not available for about three days, I will merge this PR once it's available.

Best Regards

---

From: Mikhael-Danilov @.> Sent: Tuesday, January 30, 2024 11:56:34 PM To: ThanatosShinji/onnx-tool @.> Cc: ThanatosShinji @.>; Comment @.> Subject: Re: [ThanatosShinji/onnx-tool] Improve Graph.remove_node, add Graph.remove_subtree (PR #65)

After using onnx-tool some more I came up with separate Graph.remove_dangling_nodes method, which should be called after use on remove_node or remove_subtree.

— Reply to this email directly, view it on GitHubhttps://github.com/ThanatosShinji/onnx-tool/pull/65#issuecomment-1917332636, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AZZIQRSVRQCXZS6L2MAEQMDYREJ3FAVCNFSM6AAAAABCLG6KRWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJXGMZTENRTGY. You are receiving this because you commented.Message ID: @.***>

[ThanatosShinji]

Is this PR ready for review?

[Mikhael-Danilov]

Is this PR ready for review?

Sorry, not yet.

[ThanatosShinji]

Don't mind, just tell you that I'm available to review. Take your time.

Best Regards

---

From: Mikhael-Danilov @.> Sent: Monday, February 5, 2024 5:22:35 AM To: ThanatosShinji/onnx-tool @.> Cc: ThanatosShinji @.>; Comment @.> Subject: Re: [ThanatosShinji/onnx-tool] Improve Graph.remove_node, add Graph.remove_subtree - WiP (PR #65)

Is this PR ready for review?

Sorry, not yet.

— Reply to this email directly, view it on GitHubhttps://github.com/ThanatosShinji/onnx-tool/pull/65#issuecomment-1925920338, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AZZIQRVGAKEADX6PLCSPGALYR73ZXAVCNFSM6AAAAABCLG6KRWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRVHEZDAMZTHA. You are receiving this because you commented.Message ID: @.***>

[Mikhael-Danilov]

I believe its ready now.

[ThanatosShinji]

@Mikhael-Danilov I tried remove_subtree with resnet50-v2-7, it seems that after the removal there is no output tensor.

[ThanatosShinji]

[ThanatosShinji]

Thanks! @Mikhael-Danilov