-
Notifications
You must be signed in to change notification settings - Fork 178
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.
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
onnxruntime for converted BernouilliNB, MultinomialNB (scikit-learn) does not produce the same results as the original model #151
Comments
Could you share your code and dataset? |
This is part of PR: You need to run the unit tests in class TestNaiveBayesConverter to generate model, inputs, expected outputs, all are stored on disk. The runtime is then run in class TestBackendWithOnnxRuntime. The code is in branch https://github.com/xadupre/onnxmltools/tree/testrt/onnxmltools. |
I see you have made changes to NaiveBayes.py file which has the NB converters. I had tested test_NaiveBayesConverter.py file and they were running fine. Are you saying that the original file was causing test failures or did you see the mismatch after your changes? |
When you say running, you mean just the conversion or the conversion + the execution of the converted model with one onnx backend. That's what I did today. The converter was woring fine but the execution of the converted onnx with onnxruntime was either failing either producing different results. The mismatch was before my changes. |
I mean conversion + execution. Here is what I did: from sklearn.datasets import load_iris data = load_iris() model_MNB = MultinomialNB().fit(X_train, y_train) scikit_result_MNB = np.hstack([model_MNB.predict_proba(X_test), model_MNB.predict(X_test).reshape((-1, 1))]) np.mean(onnx_res_mnb[:, 3] == scikit_result_MNB[:, 3]) # Gives 1 as output This means all the predictions match between scikit and onnx models. np.mean(np.isclose(onnx_res_mnb, scikit_result_MNB)) # Gives 1 as output MNB outputs (probabilities + labels) seem to match, whereas BNB probability values seem to vary a little although their labels match in this example. |
How did you get onnx_res_mnb? |
$ ./onnxruntime_exec.exe -m onnx_MNB.onnx -t iris_test.csv > result_MNB.csv prroy@B115FFDGPUN03 /cygdrive/c/Users/prroy/LotusRT/Lotus_Oct9/Lotus/onnxru ntime/cmake_build/Debug |
What is the version of onnxruntime you are using? I observed differences between versions. I'm currently testing against the version released on pypi (1.3.0). The tests I put in place test all outputs, prediction and probabilities. |
How do I check the version of onnxruntime? I cloned Lotus repo on Oct 9 and built onnxruntime project. |
This should work then except I think onnxruntime only produces the predicted labels and not the scores. If you build it yourself, you can add an option to build the python package too and check with this one the converted model. Documentation for onnxruntime is here: https://docs.microsoft.com/en-us/python/api/overview/azure/onnx/examples-md?view=azure-onnx-py. I'll check with the runtime tomorrow on my side. |
Here is what I get with the latest version of onnxruntime and the current onnxmltools. onnxruntime and the executable gives the same results. On this example, MNB works, BNB does not. import os data = load_iris() xt = np.zeros((X_test.shape[0], X_test.shape[1]+1)) onnxmltools.utils.save_model(onnx_MNB, "onnx_MNB.onnx") fold = r"path_to_exec" onnx_res_mnb = np.loadtxt("result_MNB.csv", delimiter=',') scikit_result_MNB = np.hstack([model_MNB.predict_proba(X_test), model_MNB.predict(X_test).reshape((-1, 1))]) print(np.mean(onnx_res_mnb[:, 3] == scikit_result_MNB[:, 3])) # Gives 1 as output #This means all the predictions match between scikit and onnx models. import onnxruntime mnb = onnxruntime.InferenceSession('onnx_MNB.onnx') bnb = onnxruntime.InferenceSession('onnx_BNB.onnx') Outputs: 0.0 |
BernouillNB - binarisation of features is not part of the converter https://github.com/scikit-learn/scikit-learn/blob/bac89c2/sklearn/naive_bayes.py#L938 |
Yup, here is the PR: #162 |
* remove unnecessary print, add quote around filenames in some places * replaces as_matrix by values (pandas warnings) * changes variable name to avoid getting warnings about invalid names * better consistency for converted, allows targetted onnx version to be None * Revert "better consistency for converted, allows targetted onnx version to be None" This reverts commit e257ca1. * handle the comparison of ONNX versions in only one place * fix bug with OneHotEncoder and scikit-learn 0.20 * release the constraint on scikit-learn (0.20.0 allowed) * fix one type issue for Python 2.7 * add documentation to compare_strict_version * Fixes #151, BernouilliNB converter * Removes unused nodes in graph * Adresses issue #143, enables build with keras 2.1.2 * Revert modifications due to a wrong merge * update keras version * Disable test on keras/mobilenet as it does not work * add unit test for xception (failing) * remove duplicate install * skip unit test if not installed (tensorflow still not available on python 3.7) * Fix when keras is not available * Fix missing import * Update test_single_operator_with_cntk_backend.py * Set up CI with Azure Pipelines * Update azure pipeline * Skip a unit test if tensorflow is not installed * merge * missing import * Revert "Merge branch 'master' of https://github.com/onnx/onnxmltools" This reverts commit 178e763, reversing changes made to 1a617ef. * revert changes * Revert changes * \r * \r * documentation * Fix appveyor * fix bad merge * remove example on keras * Delete requirements-deep.txt
* remove unnecessary print, add quote around filenames in some places * replaces as_matrix by values (pandas warnings) * changes variable name to avoid getting warnings about invalid names * better consistency for converted, allows targetted onnx version to be None * Revert "better consistency for converted, allows targetted onnx version to be None" This reverts commit e257ca1. * handle the comparison of ONNX versions in only one place * fix bug with OneHotEncoder and scikit-learn 0.20 * release the constraint on scikit-learn (0.20.0 allowed) * fix one type issue for Python 2.7 * add documentation to compare_strict_version * Fixes #151, BernouilliNB converter * Removes unused nodes in graph * Adresses issue #143, enables build with keras 2.1.2 * Revert modifications due to a wrong merge * update keras version * Disable test on keras/mobilenet as it does not work * add unit test for xception (failing) * remove duplicate install * skip unit test if not installed (tensorflow still not available on python 3.7) * Fix when keras is not available * Fix missing import * Update test_single_operator_with_cntk_backend.py * Set up CI with Azure Pipelines * Update azure pipeline * Skip a unit test if tensorflow is not installed * merge * missing import * Revert "Merge branch 'master' of https://github.com/onnx/onnxmltools" This reverts commit 178e763, reversing changes made to 1a617ef. * revert changes * Revert changes * \r * \r * first step in the migration of xgboost code * XGBoost regression works * Finalize xgboost converter * Update README.md * Add function has_tensorflow * Update test_single_operator_with_cntk_backend.py * better desgin for a unit test * update xgboost classifier * Delete test_keras_xception.py * Delete requirements-deep.txt * Delete test_keras_modebilenetv2.py * less spaces * lower precision for xgboost comparison tests * disable xgboost testing on python 2
No description provided.
The text was updated successfully, but these errors were encountered: