Try to import MO as Python package#2190
Try to import MO as Python package#2190dkurt wants to merge 5 commits intoopenvinotoolkit:developfrom
Conversation
|
@IRDonch, may we ask to review? This is done for MO installed by pip (as a part of |
IRDonch
left a comment
There was a problem hiding this comment.
You need to update the README with information about the updated MO search logic.
tools/downloader/converter.py
Outdated
| # For OpenVINO from PyPI | ||
| mo_path = subprocess.run([args.python, '-c', | ||
| 'import mo,sys; sys.stdout.write(mo.__file__)'], | ||
| check=True, stdout=subprocess.PIPE).stdout.decode('utf-8') |
There was a problem hiding this comment.
To decode as UTF-8, you should make sure that the process outputs UTF-8 (which you can do by adding env={**os.environ, 'PYTHONIOENCODING': 'utf-8'}).
There was a problem hiding this comment.
Enabled encoding='utf-8' flag
There was a problem hiding this comment.
That's good, but that doesn't do the same thing. You still need to make Python output UTF-8, and you can only do that with PYTHONIOENCODING.
tools/downloader/README.md
Outdated
| The script will attempt to locate Model Optimizer using the environment | ||
| variables set by the OpenVINO™ toolkit's `setupvars.sh`/`setupvars.bat` | ||
| script. You can override this heuristic with the `--mo` option: | ||
| script. Or trying to use PyPI distribution. |
There was a problem hiding this comment.
Or trying to use PyPI distribution.
That is not a complete sentence. It should be combined with the preceding one.
But besides that, it also needs reworking. First, it doesn't actually fit the previous sentence. If you combine them, you get "The script will attempt to locate Model Optimizer trying to use PyPI distribution", where both "attempt" and "trying" communicate the same meaning.
And second, it doesn't explain much. What PyPI distribution are you talking about? How is it used to locate Model Optimizer? I would argue that PyPI itself is actually rather irrelevant here.
There was a problem hiding this comment.
Removed PyPI mention completely. With openvinotoolkit/openvino#4801 both PyPI and and just export PYTHONPATH=/path/to/openvino/model-optimizer/:$PYTHONPATH may work
There was a problem hiding this comment.
That's a lot better, but I think you should still explicitly say that it will try to locate MO by searching for the mo package in the Python environment.
| mo_path = subprocess.run([args.python, '-c', | ||
| 'import mo,sys; sys.stdout.write(mo.__file__)'], | ||
| check=True, stdout=subprocess.PIPE, encoding='utf-8').stdout | ||
| mo_path = (Path(mo_path).parent / '__main__.py').resolve(strict=True) |
There was a problem hiding this comment.
That's not going to work correctly, because some MO arguments used for converting models refer to files inside the MO directory, and in order for them to be located correctly, mo_path must point to the mo.py script.
There was a problem hiding this comment.
Basic conversion worked for me. Do you know which arguments may be affected?
There was a problem hiding this comment.
@dkurt There are another issue with mo installation via proposed command. not all extensions are installed. For example extensions/front/tf/ssd_v2_support.json
There was a problem hiding this comment.
Issue with extensions installation has been addressed in openvinotoolkit/openvino#5302
@dkurt can you please finalize changes required to make converter work after installation of openvino-dev python package with model optimizer inside?
|
Obsoleted by #2395; closing. |
Example MO install: