Fixes #i302, handle OVR decision function #450
Conversation
xadupre
changed the title
xadupre
changed the title
|
This pull request fixes 1 alert when merging f6eca3a into 72d43c0 - view on LGTM.com fixed alerts:
|
| @@ -140,6 +148,127 @@ def convert_sklearn_svm(scope, operator, container): | |||
| raise ValueError("Unknown support vector machine model type found " | |||
| "'{0}'.".format(operator.type)) | |||
|
|
|||
| if (hasattr(op, 'decision_function_shape') and | |||
| op.decision_function_shape == 'ovr' and handles_ovr): | |||
| # Applies _ovr_decision_function. | |||
There was a problem hiding this comment.
Wouldn't it be better to just add reference to the scikit implementation rather than adding the entire code here?
There was a problem hiding this comment.
links to code are not really stable and it is easier to understand what the converter does by looking at the numpy code
| "Classes different from first n integers are not supported " | ||
| "in SVC converter.") | ||
|
|
||
| cst3 = scope.get_unique_variable_name('cst3') |
There was a problem hiding this comment.
It's difficult to understand what cst0, cst1 and cst3 mean. It would be better to have clearer node names.
There was a problem hiding this comment.
cst0 = constant equal to 0, cst1 = constant equal to 1, cst3 = constant equal to 3
There was a problem hiding this comment.
Variable name is fine, it would be better to have the onnx node name as const0, etc. Anyways, this is a minor issue.
|
This pull request fixes 1 alert when merging e5c345d into c6e631d - view on LGTM.com fixed alerts:
|
No description provided.