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.

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

Add json validate #6449

Merged
merged 43 commits into from
Jul 22, 2021
Merged

Add json validate #6449

merged 43 commits into from
Jul 22, 2021

Conversation

iimironov
Copy link
Contributor

@iimironov iimironov commented Jun 29, 2021 •
edited
Loading

#Ticket: 38301

Description:
Add json schema validator to Model Optimizer. It help to validate JSON config files so that it can catch unexpected, unreasonable input in config files.

  • Comments
  • Code style (PEP8)
  • Transformation generates reshape-able IR N/A
  • Transformation preserves original framework node names N/A: a special case, when we should rename the nodes

Validation:

  • Unit tests
  • Framework operation tests N/A
  • Transformation tests N/A
  • Model Optimizer IR Reader check N/A

Documentation:

  • Supported frameworks operations list N/A: no new operations were enabled
  • Guide on how to convert the public model N/A no new models were enabled
  • User guide update N/A

@iimironov iimironov requested review from a team, pavel-esir, rkazants, popovaan and lazarevevgeny and removed request for a team July 1, 2021 09:05
asomsiko
asomsiko reviewed Jul 1, 2021
View reviewed changes
Copy link
Contributor

@asomsiko asomsiko left a comment

Choose a reason for hiding this comment

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

Consider restricting invaliv valuesl. See comments

model-optimizer/mo/utils/schema.json Outdated Show resolved Hide resolved
model-optimizer/mo/utils/schema.json Outdated Show resolved Hide resolved
model-optimizer/mo/utils/schema.json Outdated Show resolved Hide resolved
@rkazants
Copy link
Contributor

rkazants commented Jul 2, 2021

please add description according to the template. Currently solution and root cause analysis are missed

@iimironov iimironov requested a review from asomsiko July 6, 2021 10:06
@iimironov iimironov requested a review from asomsiko July 20, 2021 08:55
@iimironov iimironov requested a review from rkazants July 20, 2021 09:26
@iimironov iimironov requested a review from rkazants July 21, 2021 11:44
lazarevevgeny
lazarevevgeny suggested changes Jul 21, 2021
View reviewed changes
model-optimizer/unit_tests/mo/utils/schema_test.py Outdated Show resolved Hide resolved
model-optimizer/unit_tests/mo/utils/schema_test.py Outdated Show resolved Hide resolved
model-optimizer/requirements.txt
@@ -8,3 +8,4 @@ onnx>=1.8.1
defusedxml>=0.7.1
urllib3>=1.26.4
requests>=2.25.1
fastjsonschema~=2.15.1
Copy link
Contributor

Choose a reason for hiding this comment

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

@generalova-kate , if we introduce new dependency should we update the openvino-dev package requirements as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

@evgenytalanin-intel , could you suggest someone to answer the question above?

model-optimizer/requirements.txt
@@ -8,3 +8,4 @@ onnx>=1.8.1
defusedxml>=0.7.1
urllib3>=1.26.4
requests>=2.25.1
fastjsonschema~=2.15.1
Copy link
Contributor

Choose a reason for hiding this comment

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

@ttroilov , could you take a look if we are ok to add this new dependency to the MO from the legal perspective?

model-optimizer/mo/utils/custom_replacement_config.py Outdated Show resolved Hide resolved
@iimironov iimironov requested a review from lazarevevgeny July 22, 2021 09:37
@iimironov iimironov requested a review from lazarevevgeny July 22, 2021 13:30
@lazarevevgeny lazarevevgeny enabled auto-merge (squash) July 22, 2021 17:13
@lazarevevgeny lazarevevgeny merged commit e1bffcc into openvinotoolkit:master Jul 22, 2021
rnugmanx pushed a commit to rnugmanx/openvino that referenced this pull request Aug 26, 2021
@iimironov
Add json validate (openvinotoolkit#6449)
451ae14 
* Add json validate

* Fix json schema

* Fix schema loader

* Add unit test

* Update bom file

* Update all requarments

* Update dev requarments

* Update requrments

* Update path to schema

* Update schema

* Add some unit tests

* Move schema to root dir

* Update schema path in bom file

* Fix unit test

* Fix bom

* Change path to schema

* update setup

* Fix setup

* Fix mo args test

* Refactoring some code

* Refactoring according to review

* Update sort imports

* Remove id attribute from schema

* Refactoring validator

* Fix according to review

* Move schema from json to dict. Update unit tests.

* Fix BOM file

* Update bom file
andrei-cv pushed a commit to andrei-cv/openvino that referenced this pull request Aug 30, 2021
@iimironov @andrei-cv
Add json validate (openvinotoolkit#6449)
5d1aa27 
* Add json validate

* Fix json schema

* Fix schema loader

* Add unit test

* Update bom file

* Update all requarments

* Update dev requarments

* Update requrments

* Update path to schema

* Update schema

* Add some unit tests

* Move schema to root dir

* Update schema path in bom file

* Fix unit test

* Fix bom

* Change path to schema

* update setup

* Fix setup

* Fix mo args test

* Refactoring some code

* Refactoring according to review

* Update sort imports

* Remove id attribute from schema

* Refactoring validator

* Fix according to review

* Move schema from json to dict. Update unit tests.

* Fix BOM file

* Update bom file
akuporos pushed a commit to akuporos/openvino that referenced this pull request Sep 29, 2021
@iimironov @akuporos
Add json validate (openvinotoolkit#6449)
decda7f 
* Add json validate

* Fix json schema

* Fix schema loader

* Add unit test

* Update bom file

* Update all requarments

* Update dev requarments

* Update requrments

* Update path to schema

* Update schema

* Add some unit tests

* Move schema to root dir

* Update schema path in bom file

* Fix unit test

* Fix bom

* Change path to schema

* update setup

* Fix setup

* Fix mo args test

* Refactoring some code

* Refactoring according to review

* Update sort imports

* Remove id attribute from schema

* Refactoring validator

* Fix according to review

* Move schema from json to dict. Update unit tests.

* Fix BOM file

* Update bom file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@popovaan popovaan popovaan approved these changes

@rkazants rkazants rkazants approved these changes

@lazarevevgeny lazarevevgeny lazarevevgeny approved these changes

@asomsiko asomsiko asomsiko approved these changes

@pavel-esir pavel-esir Awaiting requested review from pavel-esir

@vurusovs vurusovs Awaiting requested review from vurusovs

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

Successfully merging this pull request may close these issues.
6 participants
@iimironov @rkazants @asomsiko @lazarevevgeny @vurusovs @popovaan