Talip7/scikit-learn-issues-multilabel huggingface dataset:
RFC introduce methods to get and set estimators' state

Right now `clone` uses `{get, set}_params` to replicate an unfit estimator. These methods are designed to return esimators' hyperparameters. At the moment, we have no way of getting the state of a fitted estimator in a non-pickle format.

Pickle files are by design able to run arbitrary code, and therefore one should ideally only load a pickle file from a trusted source. This makes sharing and moving scikit-learn based estimators hard, which also introduces security issues when deploying ML models in production.

Another issue with pickle files is that we kinda force people to use the same versions of the libraries they used to train the model and dump the pickle. This prevents people from being able to update their base docker images when they're deploying a model which was trained a while ago, and I'm not sure if we have good ways of letting them update their pickle files for a new version.

My proposal is to introduce `{get, set}_state` methods on the `BaseEstimator` level be able to persist and set the state of models in a more portable, secure, and backward compatible way. We can probably even just do JSON.

cc @scikit-learn/core-devs @koaning

COMMENT:
I think it makes sense to have a safer representation of the model so it is easier for people to share their models without worrying about arbitrary code execution. For development, it can help us with bug reports since the user can share their actual model in the issue.

Even with `get_state` and `set_state`, we can not guarantee that a model trained on one version works on another. For example, we could have added or removed private attributes. I wouldn't want to support migrating between versions. It requires a ton of resources to support this use case.

As for implementation, I think its possible as long as we are okay with not supporting Python functions by default. We can kind of support Python callables by storing the raw python code in the serialized format, requiring the imports to be in the function itself and also allowing them to be inspected:

```python
# This Python code is stored in the serialization format
def my_sqrt(x):
    import numpy as np
    return np.sqrt(x)

# error but also show the Python code to the user (default)
est = get_state(..., error_on_raw_code=True) 

# If user is happy with the implementation, then we load the Python code:
est = get_state(..., error_on_raw_code=False)
```

Also, I think one can go a long way prototyping with an external library. `get_state` can still use the full machinery of `pickle` to generate the serialized format. The restriction is that `set_state` can not use `pickle`. (I think the hard part is designing the API to handling meta-estimators. How does one construct the DAG to `set_state` into?)

Labels: API, RFC
RFC introduce methods to get and set estimators' state

Right now `clone` uses `{get, set}_params` to replicate an unfit estimator. These methods are designed to return esimators' hyperparameters. At the moment, we have no way of getting the state of a fitted estimator in a non-pickle format.

Pickle files are by design able to run arbitrary code, and therefore one should ideally only load a pickle file from a trusted source. This makes sharing and moving scikit-learn based estimators hard, which also introduces security issues when deploying ML models in production.

Another issue with pickle files is that we kinda force people to use the same versions of the libraries they used to train the model and dump the pickle. This prevents people from being able to update their base docker images when they're deploying a model which was trained a while ago, and I'm not sure if we have good ways of letting them update their pickle files for a new version.

My proposal is to introduce `{get, set}_state` methods on the `BaseEstimator` level be able to persist and set the state of models in a more portable, secure, and backward compatible way. We can probably even just do JSON.

cc @scikit-learn/core-devs @koaning

COMMENT:
Imagine this:

```py
class Estimator(...):
    ...
    def get_state(self):
        return {sklearn.__version__, serialized(self._attr)}
```

Now if in a new version `Estimator` changes, we can see if we can construct the changed/added attributes from the ones that existed in the previous version. We can even have deprecation policies/cycles for breaking changes. I was thinking of a `set_state` which does a lot more than just setting those private attributes given the values.

> I think the hard part is designing the API to handling meta-estimators. How does one construct the DAG to set_state into?

I don't think it's complicated to get a json of a DAG. That's the easy part of the problem for me. And `set_state` would know how to interpret that.

I very much like your thinking on arbitrary code though.

Labels: API, RFC
RFC introduce methods to get and set estimators' state

Right now `clone` uses `{get, set}_params` to replicate an unfit estimator. These methods are designed to return esimators' hyperparameters. At the moment, we have no way of getting the state of a fitted estimator in a non-pickle format.

Pickle files are by design able to run arbitrary code, and therefore one should ideally only load a pickle file from a trusted source. This makes sharing and moving scikit-learn based estimators hard, which also introduces security issues when deploying ML models in production.

Another issue with pickle files is that we kinda force people to use the same versions of the libraries they used to train the model and dump the pickle. This prevents people from being able to update their base docker images when they're deploying a model which was trained a while ago, and I'm not sure if we have good ways of letting them update their pickle files for a new version.

My proposal is to introduce `{get, set}_state` methods on the `BaseEstimator` level be able to persist and set the state of models in a more portable, secure, and backward compatible way. We can probably even just do JSON.

cc @scikit-learn/core-devs @koaning

COMMENT:
The point that you are raising is a point that every large project ends
up raising.

When raising it, it is important to define the goals well enough so that
the endeavor does not end up reinventing pickle. If the goals are close
to those of pickle (general persistence), the endeavor will naturally
follow along the same path. We did this is Mayavi many years ago, and it
was a maintenance nightmare, and eventually hit the same problems that we
had with pickle.

If the goals are partial persistence only for inference, would
sklearn-onnx fullfill them?

Labels: API, RFC
RFC introduce methods to get and set estimators' state

Right now `clone` uses `{get, set}_params` to replicate an unfit estimator. These methods are designed to return esimators' hyperparameters. At the moment, we have no way of getting the state of a fitted estimator in a non-pickle format.

Pickle files are by design able to run arbitrary code, and therefore one should ideally only load a pickle file from a trusted source. This makes sharing and moving scikit-learn based estimators hard, which also introduces security issues when deploying ML models in production.

Another issue with pickle files is that we kinda force people to use the same versions of the libraries they used to train the model and dump the pickle. This prevents people from being able to update their base docker images when they're deploying a model which was trained a while ago, and I'm not sure if we have good ways of letting them update their pickle files for a new version.

My proposal is to introduce `{get, set}_state` methods on the `BaseEstimator` level be able to persist and set the state of models in a more portable, secure, and backward compatible way. We can probably even just do JSON.

cc @scikit-learn/core-devs @koaning

COMMENT:
I share @thomasjpfan 's concerns about support for backward-compatibility.

Also, (and I'm not good at asking subtle questions): is HuggingFace planning on building a scikit-learn model hub?

Labels: API, RFC
RFC introduce methods to get and set estimators' state

Right now `clone` uses `{get, set}_params` to replicate an unfit estimator. These methods are designed to return esimators' hyperparameters. At the moment, we have no way of getting the state of a fitted estimator in a non-pickle format.

Pickle files are by design able to run arbitrary code, and therefore one should ideally only load a pickle file from a trusted source. This makes sharing and moving scikit-learn based estimators hard, which also introduces security issues when deploying ML models in production.

Another issue with pickle files is that we kinda force people to use the same versions of the libraries they used to train the model and dump the pickle. This prevents people from being able to update their base docker images when they're deploying a model which was trained a while ago, and I'm not sure if we have good ways of letting them update their pickle files for a new version.

My proposal is to introduce `{get, set}_state` methods on the `BaseEstimator` level be able to persist and set the state of models in a more portable, secure, and backward compatible way. We can probably even just do JSON.

cc @scikit-learn/core-devs @koaning

COMMENT:
@GaelVaroquaux For me, the goal is to enable people to share models over the internet with less security concerns. For example, it is fairly common for researchers to share the weights of their deep learning models. `sklearn-onnx` is restricted to their subset of operators and runs on their own backend, which can give different results and have different performance characteristics. 

In this case, the maintenance cost is big enough that I think it's best to get started in a external library first.

Labels: API, RFC
RFC introduce methods to get and set estimators' state

Right now `clone` uses `{get, set}_params` to replicate an unfit estimator. These methods are designed to return esimators' hyperparameters. At the moment, we have no way of getting the state of a fitted estimator in a non-pickle format.

Pickle files are by design able to run arbitrary code, and therefore one should ideally only load a pickle file from a trusted source. This makes sharing and moving scikit-learn based estimators hard, which also introduces security issues when deploying ML models in production.

Another issue with pickle files is that we kinda force people to use the same versions of the libraries they used to train the model and dump the pickle. This prevents people from being able to update their base docker images when they're deploying a model which was trained a while ago, and I'm not sure if we have good ways of letting them update their pickle files for a new version.

My proposal is to introduce `{get, set}_state` methods on the `BaseEstimator` level be able to persist and set the state of models in a more portable, secure, and backward compatible way. We can probably even just do JSON.

cc @scikit-learn/core-devs @koaning

COMMENT:
> If the goals are partial persistence only for inference, would sklearn-onnx fullfill them?

At the moment they support a limited, albeit impressive, [subset](http://onnx.ai/sklearn-onnx/supported.html). Is it their goal to support all of sklearn? 

One issue that comes to mind with the onnx-route is that custom models, or [3rd party tools](https://scikit-lego.netlify.app/), likely won't ever be supported by their tool.

Labels: API, RFC
RFC introduce methods to get and set estimators' state

Right now `clone` uses `{get, set}_params` to replicate an unfit estimator. These methods are designed to return esimators' hyperparameters. At the moment, we have no way of getting the state of a fitted estimator in a non-pickle format.

Pickle files are by design able to run arbitrary code, and therefore one should ideally only load a pickle file from a trusted source. This makes sharing and moving scikit-learn based estimators hard, which also introduces security issues when deploying ML models in production.

Another issue with pickle files is that we kinda force people to use the same versions of the libraries they used to train the model and dump the pickle. This prevents people from being able to update their base docker images when they're deploying a model which was trained a while ago, and I'm not sure if we have good ways of letting them update their pickle files for a new version.

My proposal is to introduce `{get, set}_state` methods on the `BaseEstimator` level be able to persist and set the state of models in a more portable, secure, and backward compatible way. We can probably even just do JSON.

cc @scikit-learn/core-devs @koaning

COMMENT:
> @GaelVaroquaux For me, the goal is to enable people to share models over the internet with less security concerns.

But still: do we want to share only for inference, or full sklearn estimator?

If it's only for inference, for instance for linear models, we have much less to share. 

Once again, we need to pick up clear goals, and we need to look at alternatives and figure out why we think we can do better than those alternatives:
- general-purpose persistence: how do we avoid the problems of pickle?
- inference: why do we think that we'll do better than sklearn-onnx?

Labels: API, RFC
RFC introduce methods to get and set estimators' state

Right now `clone` uses `{get, set}_params` to replicate an unfit estimator. These methods are designed to return esimators' hyperparameters. At the moment, we have no way of getting the state of a fitted estimator in a non-pickle format.

Pickle files are by design able to run arbitrary code, and therefore one should ideally only load a pickle file from a trusted source. This makes sharing and moving scikit-learn based estimators hard, which also introduces security issues when deploying ML models in production.

Another issue with pickle files is that we kinda force people to use the same versions of the libraries they used to train the model and dump the pickle. This prevents people from being able to update their base docker images when they're deploying a model which was trained a while ago, and I'm not sure if we have good ways of letting them update their pickle files for a new version.

My proposal is to introduce `{get, set}_state` methods on the `BaseEstimator` level be able to persist and set the state of models in a more portable, secure, and backward compatible way. We can probably even just do JSON.

cc @scikit-learn/core-devs @koaning

COMMENT:
sklearn-onnx has a few issues:
- it supports a subset of the estimators
- it can easily be out of sync with the latest changes of estimators
- it's almost impossible for a third party developer to implement the converter for their own estimator into onnx
- one cannot read an onnx file into a scikit-learn estimator

So it's good IF the converter happens to support what the user has, and IF the only reason for doing it is to deploy the model.

As @thomasjpfan says, it's about sharing your model, not just deploying your model.

> Also, (and I'm not good at asking subtle questions): is HuggingFace planning on building a scikit-learn model hub?

I'm working on it 😁 

> Once again, we need to pick up clear goals, and we need to look at alternatives and figure out why we think we can do better than those alternatives:
> - general-purpose persistence: how do we avoid the problems of pickle?
> - inference: why do we think that we'll do better than sklearn-onnx?

The goal is to share the model. Inference comes as a consequence of doing that. I should be able to ask an institute/org with some private data to train a model on their data, and give me the data, and I should be able to investigate, use, and deploy that model w/o having to trust that that org/institute hasn't put anything malicious in the persisted model.

I think we can do better than the alternatives you mention because:

- We don't necessarily want general purpose persistence. We have certain requirements which would impose certain limitations on what people can persist, but that's fine.
- inference only persistent doesn't satisfy our requirements here.

Labels: API, RFC
RFC introduce methods to get and set estimators' state

Right now `clone` uses `{get, set}_params` to replicate an unfit estimator. These methods are designed to return esimators' hyperparameters. At the moment, we have no way of getting the state of a fitted estimator in a non-pickle format.

Pickle files are by design able to run arbitrary code, and therefore one should ideally only load a pickle file from a trusted source. This makes sharing and moving scikit-learn based estimators hard, which also introduces security issues when deploying ML models in production.

Another issue with pickle files is that we kinda force people to use the same versions of the libraries they used to train the model and dump the pickle. This prevents people from being able to update their base docker images when they're deploying a model which was trained a while ago, and I'm not sure if we have good ways of letting them update their pickle files for a new version.

My proposal is to introduce `{get, set}_state` methods on the `BaseEstimator` level be able to persist and set the state of models in a more portable, secure, and backward compatible way. We can probably even just do JSON.

cc @scikit-learn/core-devs @koaning

COMMENT:
Some notes from an in person discussion with @GaelVaroquaux (please correct me if I'm missing/misrepresenting certain points):

- there's a difference between a kind of estimator estate which we would consider as a *snapshot* and a kind of state which allows `warm_start`.
- persisting a snapshot is a much harder task, and it also has different requirements. For instance, for callback API, it has much less security constraints than when receiving a model from an untrusted source.
- the part of the state which is required to run `predict` and use `warm_start` requires less work, and we could consider implementing it.
- handling load of a persisted model from an older version shouldn't be in the scope, or at least originally. This item adds a significant amount of workload/CI effort to the infra and to the team.
- loading nested persisted models (like a pipeline) means the load operation will instantiate third estimators which in turn means arbitrary code execution if those instances are from other libraries/custom code. We should make sure the user explicitly allows that to happen.
- We can experiment in an external library for a while to see what it looks like and how much effort it would be.
- whatever we come up with, should not be equivalent to pickle, since we know we can't completely secure that one.
- this does not replace ONNX, and ONNX is still the right solution if people only care about inference and onnx supports the model they have.

Labels: API, RFC
RFC introduce methods to get and set estimators' state

Right now `clone` uses `{get, set}_params` to replicate an unfit estimator. These methods are designed to return esimators' hyperparameters. At the moment, we have no way of getting the state of a fitted estimator in a non-pickle format.

Pickle files are by design able to run arbitrary code, and therefore one should ideally only load a pickle file from a trusted source. This makes sharing and moving scikit-learn based estimators hard, which also introduces security issues when deploying ML models in production.

Another issue with pickle files is that we kinda force people to use the same versions of the libraries they used to train the model and dump the pickle. This prevents people from being able to update their base docker images when they're deploying a model which was trained a while ago, and I'm not sure if we have good ways of letting them update their pickle files for a new version.

My proposal is to introduce `{get, set}_state` methods on the `BaseEstimator` level be able to persist and set the state of models in a more portable, secure, and backward compatible way. We can probably even just do JSON.

cc @scikit-learn/core-devs @koaning

COMMENT:
> this does not replace ONNX, and ONNX is still the right solution if people only care about inference and onnx supports the model they have.

In particular ONNX is the only solution to support low memory overhead inference in deployment environment where a full-fledged Python distribution is not available or not wanted for any reason.

But if we are to private a standard `get_state` method in scikit-learn, that will certainly help with the maintenance of ancillary projects like sklearn-onnx.

Labels: API, RFC
RFC introduce methods to get and set estimators' state

Right now `clone` uses `{get, set}_params` to replicate an unfit estimator. These methods are designed to return esimators' hyperparameters. At the moment, we have no way of getting the state of a fitted estimator in a non-pickle format.

Pickle files are by design able to run arbitrary code, and therefore one should ideally only load a pickle file from a trusted source. This makes sharing and moving scikit-learn based estimators hard, which also introduces security issues when deploying ML models in production.

Another issue with pickle files is that we kinda force people to use the same versions of the libraries they used to train the model and dump the pickle. This prevents people from being able to update their base docker images when they're deploying a model which was trained a while ago, and I'm not sure if we have good ways of letting them update their pickle files for a new version.

My proposal is to introduce `{get, set}_state` methods on the `BaseEstimator` level be able to persist and set the state of models in a more portable, secure, and backward compatible way. We can probably even just do JSON.

cc @scikit-learn/core-devs @koaning

COMMENT:
Another use case for sklearn-to-sklearn without vulnerability to arbitrary code injection a-la pickle would be to make it possible to host a public model auditing service where you would upload a trained scikit-learn pipeline and be able to run any Python based auditing tools based on either scikit-learn's own inspection tools or third-party scikit-learn compatible tools such as [SHAP](https://github.com/slundberg/shap), [FACET](https://github.com/BCG-Gamma/facet), [ELI5](https://eli5.readthedocs.io/en/latest/), [interpretml](https://github.com/interpretml/interpret), [fairlearn](https://fairlearn.org/) and so on.

Labels: API, RFC
RFC introduce methods to get and set estimators' state

Right now `clone` uses `{get, set}_params` to replicate an unfit estimator. These methods are designed to return esimators' hyperparameters. At the moment, we have no way of getting the state of a fitted estimator in a non-pickle format.

Pickle files are by design able to run arbitrary code, and therefore one should ideally only load a pickle file from a trusted source. This makes sharing and moving scikit-learn based estimators hard, which also introduces security issues when deploying ML models in production.

Another issue with pickle files is that we kinda force people to use the same versions of the libraries they used to train the model and dump the pickle. This prevents people from being able to update their base docker images when they're deploying a model which was trained a while ago, and I'm not sure if we have good ways of letting them update their pickle files for a new version.

My proposal is to introduce `{get, set}_state` methods on the `BaseEstimator` level be able to persist and set the state of models in a more portable, secure, and backward compatible way. We can probably even just do JSON.

cc @scikit-learn/core-devs @koaning

COMMENT:
> Some notes from an in person discussion with @GaelVaroquaux (please correct me if I'm missing/misrepresenting certain points):

I think that this is a great summary of our discussion. Thanks!!

Labels: API, RFC