Add Ceph Upstream Prediction Models

[chauhankaranraj]

Hey @yaarith, I added the ceph upstream models to this repo, and updated the model.py script to use these by default.

Marking this PR as WIP because there's one piece still missing - vendor info. Is it possible to get the "vendor", "nvme_vendor" and "model_name" keys from the smartctl jsons as well? These aren't used as inputs to models, but are needed to decide which model to use (models are vendor based atm). Could you please take a look? Sorry for not mentioning this requirement earlier.

Edit: Addresses ceph#13

[yaarith]

@chauhankaranraj Great!!

I pushed the changes you asked:
4c12cc2

Please note that 'nvme_vendor' key was not added since it's the same as 'vendor'. That's because we retrieve the data from the database, where it was processed and filtered (as opposed to the raw SMART data in the client side, where 'vendor' and 'nvme_vendor' may contain different values).

Let me know if you need anything :-)

[chauhankaranraj]

Thanks for the changes, @yaarith, I have updated the predictor :)

[yaarith]

@chauhankaranraj Great, many thanks!!

I wish to test the integration, can you please add the updated requirements.txt?

[chauhankaranraj]

I wish to test the integration, can you please add the updated requirements.txt?

Of course, should be updated now.

[yaarith]

'device_data' is not defined.

I think you meant to add after line 305
device_data = json.load(inp_json)
?

[chauhankaranraj]

Oops, looks like I accidentally deleted that after testing 😅
I'll update it to include device_data = json.loads(inp_json) (coz IIUC inp_json will be a string so loads should be used instead of load)

[yaarith]

Thanks, @chauhankaranraj!

Yes, json.loads().

I still see several warnings and another error:

$ ./predict_device.py 1669 redhat
./model.py:189: FutureWarning: arrays to stack must be passed as a "sequence" type such as list or tuple. Support for non-sequence iterables such as generators is deprecated as of NumPy 1.16 and will raise an error in the future.
  means = np.vstack(gen)
./model.py:194: FutureWarning: arrays to stack must be passed as a "sequence" type such as list or tuple. Support for non-sequence iterables such as generators is deprecated as of NumPy 1.16 and will raise an error in the future.
  stds = np.vstack(gen)
./model.py:197: RuntimeWarning: invalid value encountered in true_divide
  cvs = stds / means
/usr/local/lib64/python3.6/site-packages/sklearn/utils/deprecation.py:143: FutureWarning: The sklearn.preprocessing.data module is  deprecated in version 0.22 and will be removed in version 0.24. The corresponding classes / functions should instead be imported from sklearn.preprocessing. Anything that cannot be imported from sklearn.preprocessing is now part of the private API.
  warnings.warn(message, FutureWarning)
/usr/local/lib64/python3.6/site-packages/sklearn/base.py:334: UserWarning: Trying to unpickle estimator RobustScaler from version 0.19.2 when using version 0.23.2. This might lead to breaking code or invalid results. Use at your own risk.
  UserWarning)
Traceback (most recent call last):
  File "./model.py", line 326, in <module>
    sys.exit(main())
  File "./model.py", line 319, in main
    prediction_result = predictor.predict(device_data)
  File "./model.py", line 264, in predict
    preprocessed_data = self.__preprocess(disk_days, manufacturer)
  File "./model.py", line 212, in __preprocess
    featurized = scaler.transform(featurized)
  File "/usr/local/lib64/python3.6/site-packages/sklearn/preprocessing/_data.py", line 1259, in transform
    X -= self.center_
ValueError: operands could not be broadcast together with shapes (161,70) (67,) (161,70)

[chauhankaranraj]

@yaarith I think most of the warnings are expected future deprecation warnings. But the last UserWarning is something I haven't seen before - are the packages installed according to the requirements? As per requirements.txt, scikit-learn needs to be 0.19.2, but from the output it seems like 0.23.2 is being used?

Could you please lmk how I can replicate the last ValueError? I tried running ./predict_device.py 1669 redhat but I don't have the grafana.dsn file so I get an error. So then I ran ./model.py --predictor-model redhat but that didn't throw this error.

Note: for testing, I had commented out the stdin.read() and loaded the sample input as follows:

# inp_json = sys.stdin.read()
# device_data = json.loads(inp_json)
fname = 'input_samples/predict_1669.json'
with open(fname, 'rb') as f:
    device_data = json.load(f)

[yaarith]

I think most of the warnings are expected future deprecation warnings

These warnings will become errors soon, and I'd rather have this code stable for a while :-)

As per requirements.txt, scikit-learn needs to be 0.19.2, but from the output it seems like 0.23.2 is being used?

Indeed, 0.23.2 is used. I think this issue relates to the issues Ceph users are having with diskprediction local module. Users who still have version 0.19.2 will eventually upgrade scikit-learn and the module will be broken. Also, the latest version in Ceph is 0.23.2, so we want to keep it on par with that. Is it possible to make it work with the latest scikit-learn version?

Could you please lmk how I can replicate the last ValueError?

You can run:
$ cat ./input_samples/predict_1669.json | ./model.py --predictor-model redhat
which saves you the changes between testing and production versions (no need to add the open file code block).
When I ran the code like this I see the exact same error.

[chauhankaranraj]

In this case I prefer we move on to refining the time and confidence granularity of the existing model /
training the new model. So we can use what we have now, then replace it with an improved model later.

Yep, that's exactly what I had in mind.

Regarding ProphetStor: since we will not retrain the model it should not be a problem.

Sounds good!

1. Please remove any logic from model.py which is not related to 'redhat' model; we will add different models in separate files later.

Cool, shall I remove the placeholder function simple_prediction as well then?

[yaarith]

Yep, that's exactly what I had in mind.

Can we make sure we train using the latest versions of numpy, scikit-learn, etc.? These models serve Ceph upstream and we want to move forward to the latest releases.

Cool, shall I remove the placeholder function simple_prediction as well then?

Yes, you can remove anything in this file which is not related to 'redhat' model (so anything which was part of the original placeholder model). The idea is that each model will be in a separate file.

The output should go to stdout, please remove the comment from print(prediction_result) (line 303 in model.py).

Oh, I meant to uncomment the line :-)

[chauhankaranraj]

Can we make sure we train using the latest versions of numpy, scikit-learn, etc.? These models serve Ceph upstream and we want to move forward to the latest releases.

Definitely. IIRC when we trained these models last year, the constraint sklearn==0.19 already existed, so we didn't want to break the existing models by updating it. But since that constraint is not relevant here, we'll use the latest and greatest now :)

Yes, you can remove anything in this file which is not related to 'redhat' model (so anything which was part of the original placeholder model). The idea is that each model will be in a separate file.

Oh, I meant to uncomment the line :-)

Pushed these changes 👍

[yaarith]

Thanks!

But since that constraint is not relevant here, we'll use the latest and greatest now :)

Sounds good. We do need to understand how we integrate them with future Ceph releases, so the diskprediction module uses them and does not break.

Does model.py need the SMART reports to be consecutive?
So if we have 6 reports with SMART data, but there's a gap between them - will it affect the results in any way?

[chauhankaranraj]

Does model.py need the SMART reports to be consecutive?
So if we have 6 reports with SMART data, but there's a gap between them - will it affect the results in any way?

It won't break, but the output may not be accurate in this case. The model has been trained to predict health based on exactly last 6 days of device behavior. So if the input is data from days other than the last 6 days, then we can't be sure that the result will be meaningful.

[yaarith]

Can also remove this ProphetStor reference