-
Notifications
You must be signed in to change notification settings - Fork 163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Data preprocessing and information leakage #48
Comments
Hi Hector, thank you for reaching out and for sharing your suggestions. |
Let me try over the week replacing the models, regressor/classifier, with a pipeline model including one standardization step. If this works it can be made a kwarg in predictors. |
I would like to protect your time, so before you start implementing the proposal, please provide a concept (aka some examples) for the API first. This way, we can first discuss the new API (aka user experience) and when we agree on a suitable API, we can talk about the implementation. |
Yes, Florian, minum changes if it works, could be a keyword argument addition for a list of transformations in the predictors function that reaches the VALID_CALCULATIONS dictionaries and replaces tree.DecissionTree*() with a a pipeline preprocessing using the input list of transformations. According to this: |
I think I got it - can you still please give 1 detailed example with the actual syntax. I would love to have a look at how the total code would look like |
As example only, in calculation: from sklearn.preprocessing import StandardScaler #Change the models to: VALID_CALCULATIONS = { This provides slightly different results for some data sets. The idea is to enable "predictors" function to alter the model keys to a constructed pipeline, whose constructor is a little bit awkward as it is a tuple (name, transformer). The pipeline should take care of non-leaking cross validation scores. A call to predictors would look like: transformers = [StandardScaler(), MinMaxScaler()] Here predictors (or other function) would have to create the pipeline list. |
Hi Hector, thank you for the example, and I like the transformers API When I thought about this proposal, I was unsure which problem this should solve exactly? What is the scenario that the user is in and why does the user use ppscore in that scenario? |
Hello Florian, As a sideline, the cv object could also be defined as kwarg in the predictors functions to accept other splits, stratified k folds comes to mind for very unbalanced datasets. These are the two operations I had to manually perform in this case, kwarging transformations and cv object can automate it and make the PPS more flexible. This can generate quick checks, PPS_standard is pps with the pipeline added:
|
Thank you for the explanation. Did you generate a cross-validation object at the end of your pipeline? |
Hello Florian, sklearn pipe requires the last element to be the estimator, which in PPS will be the automated choice of regression or classifier, so I have not found any other way to feed it in but to overwrite the whole model with the pipe that has the original estimator as last element. The whole pipeline could be and input to PPS, the user would have to decide regression or classification in this case, or complicate the logic of _determine_case_and_prepare_df so that it selects, from multiple models that are either classifiers or regressors. This would allow on the other hand to compare PPS using multiple different models, not only tree. |
Hi Hector, I wish you a good start to the new year and sorry for the late reply - I have been on vacation. Thank you for clarifying the point that the model is the last step for the crossvalidation object and thus it is not possible to pass the full cv object. If you want you can go ahead and add a PR |
Happy New Year Florian. I will open the PR and propose the changes. |
Hello Florian, This brings no noticeable computational cost but it cannot pass this test: line 156 of tests: As the model object at the 'model' entry of the dictionary is a different instance of a model with the same parameters. The contents are the same in every other entry of the dict, the model is not and fails to assert. This test (and subsequent result comparisons) could be modified to compare the dict excluding the 'model' entry, just as a suggestion. |
Thank you for the heads-up. We can easily adjust that test |
Hello, before anything thanks for the package, it is very useful and the overall approach is innovative and generates a lot of efficiency. I have a comment regarding the "state" of the data to run the pps analysis on, it seems (I may be mistaken) that any transformation to the data (standardization for example) will lead to large data leakage into the Kfolds cross-validations. Is it correct? The module could use sklearn´s pipelining and standard transforms to possibly increase the information generated, would this be of value to the module?
The text was updated successfully, but these errors were encountered: