Skip to content
This repository has been archived by the owner on May 31, 2024. It is now read-only.

Gorgonia backend does not handle model with several root nodes #48

Closed
mattn opened this issue May 9, 2019 · 4 comments
Closed

Gorgonia backend does not handle model with several root nodes #48

mattn opened this issue May 9, 2019 · 4 comments
Labels
backend Feature request Gorgonia / Gorgonnx This issue is related to the Gorgonia backend

Comments

@mattn
Copy link
Collaborator

mattn commented May 9, 2019

Is your feature request related to a problem? Please describe.

I'm trying to run ssd using onnx-go. But error occured.

https://gist.github.com/mattn/f5aa1b96753c76075f2666235919a8f3

2019/05/09 11:09:55 onnx: operator  not implemented ()

Describe the solution you'd like

As far as I looked code backend/x/gorgonnx, the error seems occur at here:

if len(g.roots) != 1 {
return &onnx.ErrNotImplemented{}
}

Describe alternatives you've considered

Sorry, I can't figure out what should do here.

@owulveryck owulveryck added this to To Do in Execute the SSD model via automation May 9, 2019
@owulveryck owulveryck added the bug Something isn't working label May 9, 2019
@owulveryck
Copy link
Owner

owulveryck commented May 9, 2019

Good catch!

The model has multiple "root" nodes which is not handled yet by the backend. This is an extract of the model rendered with netron:

ssd

Two things to do:

  • setting a more explicit error message
  • implement the feature

I will split this into two issues.

@owulveryck
Copy link
Owner

I am changing the title of this issue; I have created a "project" to follow the implementation steps to make the SSD model working.

Note: I have analyzed the model, and it will require a lot of effort to make it work with gorgonia.

@owulveryck owulveryck changed the title SSD does not work Give a more explicit error message when imported a multi-root model within Gorgonia May 9, 2019
@owulveryck owulveryck changed the title Give a more explicit error message when imported a multi-root model within Gorgonia Gorgonia backend does not handle model with several root nodes May 9, 2019
@owulveryck owulveryck added backend Feature request Gorgonia / Gorgonnx This issue is related to the Gorgonia backend and removed bug Something isn't working labels May 12, 2019
@owulveryck
Copy link
Owner

There is a work-in-progress with the branch issue-48 but it needs some more investigation.

The actual design of "gorgonnx" does not handle operators with multiple output.

the signature of the apply method will probably require some changes:

type operator interface {
        // apply analyse the graph to find the children of the node
        // then extract its gorgonia.Node references
        // and assign the result of the operation to the node n
        apply(g *Graph, n *Node) error
        // init the operator with name and attributes as carried by the onnx.Operator
        init(o onnx.Operation) error
}

@owulveryck
Copy link
Owner

owulveryck commented Aug 27, 2019

PR #115 fixes this error, but it introduces a breaking change.
The OperationCarrier interface needed to be changed to handle nodes with multiple outputs.

onnx-go/backend.go

Lines 19 to 27 in f9baf80

// OperationCarrier should be a method of the graph
// because the operation needs the topology of the graph
// to check the arity of the node for example
type OperationCarrier interface {
// ApplyOperation on the graph nodes
// graph.Node is an array because it allows to handle multiple output
// for example a split operation returns n nodes...
ApplyOperation(Operation, ...graph.Node) error
}

This will require to change any backend implementation.
As far as I know, the only backend implemented so far is Gorgonia (via gorgonnx), and it has been patched.

Execute the SSD model automation moved this from To Do to Done Aug 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backend Feature request Gorgonia / Gorgonnx This issue is related to the Gorgonia backend
Projects
Development

No branches or pull requests

2 participants