[SYSTEMDS-3256] Adding include vector parameter in cleaning primitives#1490
[SYSTEMDS-3256] Adding include vector parameter in cleaning primitives#1490Shafaq-Siddiqi wants to merge 4 commits into
Conversation
-This commit introduces an include vector parameters in cleaning primitives this vector indicates the rows that should be considered in building the model. It makes a distinction between train and test dataset i.e., which rows should be a part of model learning and which rows should only be considered for applying the model but are not a part of the model. TODO: create a builtin for entropyTransformer and frequencyTransformer and remove them from the executePipeline.dml script
|
Thanks for the continued effort on the cleaning framework @Shafaq-Siddiqi. After going through this PR, I had the feeling this splitting into train/test within the individual cleaning primitives is not at the right place. Instead of constructing the include vector during pipeline execution and splitting in each primitive, we could simply split the dataset before. In most primitives (let's take normalize() as an example) the state handling of colMins and colMaxs did not change at all, so upfront splitting would yield the same results. If I'm missing something, please let me know. Coming back to state management, I would propose the following:
Would that work for you? |
Yes, this would work in general with all the primitives except the complex ones like mice. I will start working on the library idea as for enumeration we need wrappers with takes train and test datasets and internally call normalize and then normalizeApply inside a single box (i.e., function definition.) |
-This commit introduces an include vector parameters in cleaning primitives
this vector indicates the rows that should be considered in building the model.
It makes a distinction between train and test dataset i.e., which rows should be
a part of model learning and which rows should only be considered for applying
the model but are not a part of the model.
TODO: create a builtin for entropyTransformer and frequencyTransformer and remove them
from the executePipeline.dml script