Hummingbird integration with lgbm converter #404
Conversation
Add additional tests for Hummingbird
|
|
|
@interesaaat is there an update regarding this issue? |
|
@fgsilva the PR is ready, I am waiting for review. |
.azure-pipelines/win32-conda-CI.yml
Outdated
| @@ -68,6 +68,7 @@ jobs: | |||
| echo Test numpy installation... && python -c "import numpy" | |||
| python -m pip install %COREML_PATH% %ONNX_PATH% | |||
| python -m pip install tensorflow-cpu==1.15.0 | |||
| python -m pip install torch==1.5.0+cpu -f https://download.pytorch.org/whl/torch_stable.html | |||
There was a problem hiding this comment.
onnxmltools doesn't have any dependency on pytorch, and if it is hummingbird dependency, it should be included in hummingbird setup script.
There was a problem hiding this comment.
Agreed! Pytorch dependency is actually in Hummingbird requirements, however we found that pytorch is not update on pip for windows. Another option is to install hummingbird directly from source rather than from pip. That would work. Should I install Hummingbird from github?
There was a problem hiding this comment.
In that case, can we put this line in the hummingbird requirements.txt : torch==1.5.0+cpu -f https://download.pytorch.org/whl/torch_stable.html,
Or add some lines in the code to log this issue or avoid the confusion?
There was a problem hiding this comment.
This is complicated because torch==1.5.0+cpu -f https://download.pytorch.org/whl/torch_stable.html will not work for linux and mac. We have this logic to install PyTorch, but we are having problems with pip from windows. I think that installing directly from github is probably the best solution right now.
There was a problem hiding this comment.
Ok installing from github does not work for python 3.7 because of this. Basically onnx uses typing which for some reason creates problem with Hummingbird. Since you are already manually installing tensorflor, is it a problem if for windows we also manually install pytorch in the pipeline?
| @@ -17,7 +18,7 @@ | |||
|
|
|||
| def convert(model, name=None, initial_types=None, doc_string='', target_opset=None, | |||
| targeted_onnx=onnx.__version__, custom_conversion_functions=None, | |||
| custom_shape_calculators=None): | |||
| custom_shape_calculators=None, onnx_operators_only=False): | |||
There was a problem hiding this comment.
A more accurate name is without_onnx_ml = True
There was a problem hiding this comment.
Yes! Let me rename it.
onnxmltools/convert/common/utils.py
Outdated
| @@ -5,3 +5,15 @@ | |||
| # -------------------------------------------------------------------------- | |||
|
|
|||
| from onnxconverter_common.utils import * # noqa | |||
|
|
|||
|
|
|||
| def hummingbird_installed(): | |||
There was a problem hiding this comment.
please move this func into https://github.com/microsoft/onnxconverter-common/blob/master/onnxconverter_common/utils.py.
this file will be deprecated.
remove hummingbird_installed from utils point to the onnxconveter-common branch with the hummingb_installed code
trigger pipeline
I merged the PR in onnxconverter-common so now you can update your final version and I will merge this PR then. |
|
Thanks! I am working on it right now. Will push in a minute. |
Closes #403.