Add SGD regressor

[andrei-stoian-zama]

Closes https://github.com/zama-ai/concrete-ml-internal/issues/4093

[bcm-at-zama]

Thx a lot, this was quick.

Few questions:

• we don't add the classifier at the same time?
• Plus, I think there is a few places in the doc to add mention of the new model, like, here.
• plus, nothing to change on the test side?

[RomanBredehoft]

thanks for this, though there are a few missing changes + maybe create an issue for the workaround !

[bcm-at-zama]

@RomanBredehoft : how was coverage 100%, if you say that @andrei-stoian-zama missed few things in tests?

[kcelia]

@andrei-stoian-zama, I think you must update:

• tests/common/test_skearn_model_lists.py::test_sklearn_args | test_models_and_datasets | test_get_sklearn_models-> update the new number of models that we have + generated datasets (assert test_counter == 20, ...)
• tests/sklearn/test_dump_onnx.py -> Funny exercise, where you have to "verifier si le model a une bonne tronche" lol
• add it to _classifier_models in src/concrete/ml/pytest/utils.py
• import it in tests/common/test_skearn_model_lists.py
• add a tolerance threshold to check_correctness_with_sklearn in tests/sklearn/test_sklearn_models.py
• import it in src/concrete/ml/sklearn/init.py from .linear_model import ElasticNet, Lasso, LinearRegression,...

If you encounter a flaky with: tests/sklearn/test_sklearn_models.py::test_separated_inference for KNeighborsClassifier, it's normal. https://github.com/zama-ai/concrete-ml/actions/runs/6575120273/job/17863923434?pr=372 has to be merged first.

[bcm-at-zama]

@RomanBredehoft : how was coverage 100%, if you say that @andrei-stoian-zama missed few things in tests?

ok my bad. Thanks to is_public_cml_model, the models are called but yes, pytest are red as explained by @RomanBredehoft / @kcelia

[bcm-at-zama]

I had in mind coverage 100% => tests are ok, since often test ko => coverage < 100%, but it's not always the case, as shown here

[RomanBredehoft]

yeah the model is still called and testsed, it's just that some tests need to consider it manually ! and it's intentional btw, to make sure everything is expected when adding new models (at least for the dump and test_sklearn_list tests)

[bcm-at-zama]

and it's intentional btw, to make sure everything is expected when adding new models (at least for the dump and test_sklearn_list tests)

yes, I like the fact that we check we really test the new models. Things which are too automatic can easily have mistakes we don't see

[kcelia]

I update my previous message

[kcelia]

You haven't added it to src/concrete/ml/sklearn/init.py

[kcelia]

Thanks!

[RomanBredehoft]

great thanks !

[github-actions]

⚠️ Known flaky tests have been re-run ⚠️

One or several tests initially failed but were detected as known flaky tests. They therefore have been re-run and passed. See below for more details.

Failed tests details

Known flaky tests that initially failed:

• tests/torch/test_brevitas_qat.py::test_brevitas_tinymnist_cnn[True-True-3]

[github-actions]

Coverage passed ✅

Coverage details

---------- coverage: platform linux, python 3.8.18-final-0 -----------
Name    Stmts   Miss  Cover   Missing
-------------------------------------
TOTAL    6306      0   100%

51 files skipped due to complete coverage.