Skip to content

Fix output directory of test #117

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Mar 1, 2019
Merged

Fix output directory of test #117

merged 11 commits into from
Mar 1, 2019

Conversation

disktnk
Copy link
Member

@disktnk disktnk commented Feb 28, 2019 •
edited
Loading

after

out/opset7
├── test_absolute
│   ├── model.onnx
│   └── test_data_set_0
│       ├── input_0.pb
│       └── output_0.pb
├── test_add
│   ├── model.onnx
│   └── test_data_set_0
│       ├── input_0.pb
│       ├── input_1.pb
│       └── output_0.pb
├── test_addconstant
│   ├── model.onnx
│   └── test_data_set_0
│       ├── input_0.pb
│       └── output_0.pb
├── test_average_pooling_2d
...

Apply only function test, I'll do it for test_rt.py and test_inout.py on next PR.

@disktnk disktnk force-pushed the fix/test-unique-out branch from dc72463 to 55d9284 Compare March 1, 2019 03:24
@disktnk disktnk marked this pull request as ready for review March 1, 2019 03:34
@disktnk disktnk force-pushed the fix/test-unique-out branch from 7dc4b83 to c28ab96 Compare March 1, 2019 03:55
shinh
shinh approved these changes Mar 1, 2019
View reviewed changes
Copy link
Member

@shinh shinh left a comment

Choose a reason for hiding this comment

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

This is great! Thanks for doing this.

onnx_chainer/testing/test_onnxruntime.py Outdated

def gen_test_data_set(model, args, name, opset_version):
model.xp.random.seed(42)
test_path = os.path.join(
Copy link
Member

Choose a reason for hiding this comment

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

Just a note: I don't think we should do something immediately, but I guess now we output a few giga bytes to "out" directory? Perhaps it's better to auto-remove some ONNX models generated by test_rt.py. For example, I'd keep large ONNX models only for the most recent opset_version.

Copy link
Member Author

Choose a reason for hiding this comment

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

agree, pytest framework support command args, for example, add pytest --keep-models=False option easily (maybe), I'll survey these options. Thanks.

shinh
shinh approved these changes Mar 1, 2019
View reviewed changes
Copy link
Member

@shinh shinh left a comment

Choose a reason for hiding this comment

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

lgtm

@disktnk disktnk mentioned this pull request Mar 1, 2019
Merged
@disktnk disktnk merged commit 5d36f8e into chainer:master Mar 1, 2019
@disktnk disktnk deleted the fix/test-unique-out branch March 1, 2019 09:37
@disktnk disktnk added this to the 1.3.3 milestone Mar 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shinh shinh shinh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.3.3
Development

Successfully merging this pull request may close these issues.
2 participants
@disktnk @shinh