mboehm7 edited a comment on pull request #1490: URL: https://github.com/apache/systemds/pull/1490#issuecomment-1002285230
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: * I'll add an evalList function that returns a list if a called function has multiple returns, that way we can reuse existing primitives without changes. * During pipeline enumeration, we then keep track of these state bundles and return them together with the top-k cleaning pipelines * We should further clearly separate between fitPipeline and applyPipeline and per primitive the frame that encodes primitives indicates the number and/or position of state variables. * We have pairs of fit (e.g., normalize) and apply (normalizeApply) functions where the former can call the latter as part of its implementation. If it helps to groups these pairs we can think of a library similar to our NN library that gets imported during pipeline enumeration. Would that work for you? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
