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

How to regenerate the onnx tests? #128

Open
diegobernardes opened this issue Sep 17, 2019 · 7 comments
Open

How to regenerate the onnx tests? #128

diegobernardes opened this issue Sep 17, 2019 · 7 comments
Labels
question Further information is requested

Comments

@diegobernardes
Copy link
Collaborator

Is your feature request related to a problem? Please describe.
I've updated the test template and now I need to regenerate the tests.

Describe the solution you'd like
Would be nice to have a command like make gen-tests or something like go generate ./... that could run those commands.

@diegobernardes diegobernardes added the question Further information is requested label Sep 17, 2019
@owulveryck
Copy link
Owner

owulveryck commented Sep 17, 2019

There is a gen_cmd here.
It is badly written (it would need a complete rewrite I think. It is hard to maintain) but it does the job. You need to have a local copy of the onnx repo to generate the tests.

By now, there is no plan to update the tests periodically, maybe we should think about it.

@diegobernardes
Copy link
Collaborator Author

I can do that, maybe we can abstract everything inside a Docker image. Didn't knew that I had to pull the onnx repository as well. How the current tests were generated? Was one by one? Because I don't also know what are all the operations that onnx-go supports.

@owulveryck
Copy link
Owner

owulveryck commented Sep 18, 2019

I apologize, I should have written the command line to generate the tests.

Here it is:

go run *.go -op "*" -testpath /path/to/onnx/onnx/backend/test/data/node/ -outputdir ../

You can replace the wildcard by an operator name, and get rid of the outputdir flag to dump the result on stdout.

I don't think we need docker in this case. Maybe we can just mention that in a readme somewhere.

@diegobernardes
Copy link
Collaborator Author

diegobernardes commented Sep 18, 2019

Never mind the Docker, I was thinking in another stuff and got confused 😅
I'm gonna improve this auto generation test.

I didn't found any thing related to what version of onnx the onnx-go supports. Does onnx versions are backward compatible, like Go? I'm asking this because with the master and with the last release, v1.5.0, I'm having errors at the test, things like this:

➜  onnx-go git:(fix-tests) ✗ ./go.test.sh
ok  	github.com/owulveryck/onnx-go	1.169s	coverage: 65.7% of statements
?   	github.com/owulveryck/onnx-go/backend	[no test files]
?   	github.com/owulveryck/onnx-go/backend/simple	[no test files]
ok  	github.com/owulveryck/onnx-go/backend/testbackend	1.153s	coverage: 1.6% of statements
ok  	github.com/owulveryck/onnx-go/backend/testbackend/onnx	2.444s	coverage: 100.0% of statements
?   	github.com/owulveryck/onnx-go/backend/testbackend/onnx/gen_cmd	[no test files]
ok  	github.com/owulveryck/onnx-go/backend/testbackend/testreport	1.171s	coverage: 50.0% of statements
--- FAIL: TestONNX (0.26s)
    --- FAIL: TestONNX/TestReshapeOneDim (0.00s)
        test_structure.go:121: Cannot reshape, bad output shape 24
--- FAIL: TestReshape_Scalar (0.00s)
    reshape_test.go:43: Cannot reshape, bad output shape []float32{0, 1, 2, 3, 10000, 10001, 10002, 10003}
FAIL
coverage: 72.1% of statements
FAIL	github.com/owulveryck/onnx-go/backend/x/gorgonnx	1.109s
FAIL

This is the kind of stuff that we don't need to run everytime, but when we do, it must be "reproducible" and easy as possible. My plan is to use something like go-git to download the onnx repository into a temporary folder, checkout the correct version (commit or tag), generate the code and then cleanup the temporary folder. With that we can expose the code generation through a go generate ./... command and the developer don't gonna need to care about how to use the command, if it has the onnx code and if it's in the correct version.

Sounds good to you @owulveryck?

@owulveryck
Copy link
Owner

owulveryck commented Sep 18, 2019

Sounds like a very good idea. The problem is that the repository will have one more dependency. But by now, we can deal with it I guess.

Besides that, what you say is a good point about the version onnx-go should support.
By now, there is no test on the OPset or any other version. We should add it.

I really like the idea of the Go compatibility, but I have no idea of the effort needed to maintain such compatibility policy. I'd rather evaluate the risk before claiming it.
It may come with a v1 version of the onnx-go API i guess.

PS: Sorry about the spam (I mean the previous comment I have deleted), the first reply was off-topic, I misread your comment

@diegobernardes
Copy link
Collaborator Author

Sounds like a very good idea. The problem is that the repository will have one more dependency. But by now, we can deal with it I guess.

We already have this dependency, the only thing that I'm proposing is to embrace and automate to make it easier to others. And it will work just like it's working today, the generated tests will be committed.

Being new in a project is really good to detect this kind of pain points.

Besides that, what you say is a good point about the version onnx-go should support.
By now, there is no test on the OPset or any other version. We should add it.

I really like the idea of the Go compatibility, but I have no idea of the effort needed to maintain such compatibility policy. I'd rather evaluate the risk before claiming it.
It may come with a v1 version of the onnx-go API i guess.

The lack of knowledge on the supported ONNX version is bad for everyone, to the users that are trying to use the package and for us as we have a moving target on the tests. For now I think I'll point to the last release and later on when we have a more clear vision of what the project need to support or not we can revisit this point.

PS: Sorry about the spam (I mean the previous comment I have deleted), the first reply was off-topic, I misread your comment

Don't worry 😄

@owulveryck
Copy link
Owner

We may also have a look at https://arxiv.org/abs/1906.05676v1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants