[ENH] enable array API support in LogisticRegression#2941
[ENH] enable array API support in LogisticRegression#2941avolkov-intel wants to merge 3 commits intouxlfoundation:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 30 files with indirect coverage changes 🚀 New features to boost your workflow:
|
| @supports_queue | ||
| def fit(self, X, y, queue=None): | ||
|
|
||
| # Is sparsity check here fine? - Same in BasicStatistics |
There was a problem hiding this comment.
Answer is: yes, provided that the sklearnex validation already happened before.
| dtype=[np.float64, np.float32], | ||
| ) | ||
| y_proba = self._onedal_predict_proba(X, queue) | ||
| xp, is_array_api_complient = get_namespace(X) |
There was a problem hiding this comment.
| xp, is_array_api_complient = get_namespace(X) | |
| xp, is_array_api_compliant = get_namespace(X) |
| min_prob = 1e-15 | ||
| max_prob = 1.0 - 1e-15 | ||
|
|
||
| # TODO ARRAY API only branch check how this code can be adapted |
There was a problem hiding this comment.
What do you mean "adapted"? This already works with array API, and there is a scikit-learn method that can work on CPU with the fitted coefficients from oneDAL when they are on CPU.
| accept_large_sparse=_sparsity_enabled, | ||
| dtype=[np.float64, np.float32], | ||
| ) | ||
| xp, is_array_api_complient = get_namespace(X) |
There was a problem hiding this comment.
| xp, is_array_api_complient = get_namespace(X) | |
| xp, is_array_api_compliant = get_namespace(X) |
|
|
||
| def _onedal_predict(self, X, queue=None): | ||
| if queue is None or queue.sycl_device.is_cpu: | ||
| #TODO modify function to return array api compliant results??? |
There was a problem hiding this comment.
No, if the input is not supported, it should be offloaded to scikit-learn, which will likely error out on its own. Either way, daal4py makes such a check for whether to offload to scikit-learn or not:
So it's safe to not modify the output here.
|
|
||
| def _onedal_score(self, X, y, sample_weight=None, queue=None): | ||
| # TODO1 - OK | ||
| # is accuracy_score array api compatible? Seems it is functions from skelarn it's ARRAY API compatible |
There was a problem hiding this comment.
Yes, you can check for compatibility in their documentation:
https://scikit-learn.org/stable/modules/array_api.html#metrics
|
|
||
| # TODO do we need to support behavior when model fitted with sklearn | ||
| # (e.g. torch tensor or else and then this method is run) | ||
| # Currently it can't |
There was a problem hiding this comment.
It should be possible, because the oneDAL model object would only require coefficients and intercepts.
|
|
||
| self._onedal_model = result.model | ||
|
|
||
| # For now it's fine to keep n_iteration as numpy variable |
There was a problem hiding this comment.
I guess the answer should be yes for scikit-learn compatibility, since they do not fully support array API for logistic regression at the moment. But that might change in the future.
| # Is sparsity check here fine? - Same in BasicStatistics | ||
| is_csr = _is_csr(X) | ||
|
|
||
| # Is it good place? - Same in LinReg |
There was a problem hiding this comment.
Yes, but remember that this attribute also needs to be present in the sklearnex object, because sklearn has it.
| dtype=[np.float64, np.float32], | ||
| ) | ||
|
|
||
| xp, is_array_api_complient = get_namespace(X) |
There was a problem hiding this comment.
| xp, is_array_api_complient = get_namespace(X) | |
| xp, is_array_api_compliant = get_namespace(X) |
|
@avolkov-intel This will also require updates to the documentation for array API here: Since LogisticRegression will be a special case that won't work like the rest and won't meet some of the specifications mentioned in that doc - for example: |

Description
Checklist:
Completeness and readability
Testing
Performance