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
GaussianProcessRegressor with float and double in ONNX models #220
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR takes the coverage numbers from 87% to 82%. Could you add relevant unit tests?
docs/examples/plot_gpr.py
Outdated
|
||
import skl2onnx | ||
import onnxruntime | ||
import onnx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would look better/more readable if we have these in alphabetical order and all sklearn imports clubbed together.
skl2onnx/_parse.py
Outdated
this_operator.outputs.append(mean_tensor) | ||
|
||
if options['return_std'] or options['return_cov']: | ||
# covarance or standard deviation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
covariance spelling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
@@ -0,0 +1,255 @@ | |||
# ------------------------------------------------------------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add unit tests for this file. Current coverage is 12%.
skl2onnx/operator_converters/_gp_kernels.py 121 107 12% 23-24, 28-61, 66-73, 78-88, 97-104, 113-121, 126-232, 238-255
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit tests do exists. They are just disabled since they cannot be run with the current runtime.
@@ -0,0 +1,126 @@ | |||
# ------------------------------------------------------------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add unit tests to improve coverage for this file? It's just 18%.
skl2onnx/operator_converters/gaussian_process.py 56 46 18% 15-16, 36-121
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same answer as above.
No description provided.