Closed HeuristicLab-Trac-Bot closed 7 years ago
- created a folder for all classes related to transformation from and to trees
- created a transformator which takes a tree and uses AutoDiff to produce a function and gradient function for the tree.
- moved code from SymbolicRegressionConstantOptimizationEvaluator to TreeToAutoDiffTermTransformator to make AutoDiff for trees more accessible
- renaming of folder "Transformation" to "Converters" to distinguish between transformations for variables (from data preprocessing) and classes for transformation of trees.
- renamed SymbolicDataAnalysisExpressionTreeSimplifier -> TreeSimplifier
- Implemented a converter to create a linar model as a symbolic expression tree
- extended converter for linear models to support lagged variables and changed AR(k) to use this method
Todo: remove class scaling (needs persistence backwards compat in GPR)
- removed AlglibUtil.cs and added an extension method .ToArray(names, rows) to IDataset instead.
- refactored transformation so that it is possible to apply an transformation without resetting the parameters
- Used transformations instead of Scaling as far as possible.
- Moved TakeEvery extension method to HL.Common
r14396: added methods to get training and test input matrices from ProblemData
This is needed by #745.
However, #2650 should be merged to the trunk first before we apply the changes reverted in r14400 because there are many conflicting changes.
r14507: Added caching of parameters and removed resetting of training and test partition in AdjustProblemDataProperties (used for changing the problem data of a solution) in DataAnalysisProblemData.
r14854: introduced original code for scaling from AlglibUtil into GaussianProcessModel to guarantee backwards compatibility regarding unit test results
2697: Made symbol fields readonly in TreeSimplifier.
Review comments
Enumerable Extensions
- ~~ Why has the ext. method TakeEvery been added, although it is never used? ~~ \ gkronber: Good question. I traced this back to r7154. Seems we never needed this. Can be removed.\ mkommend: Addressed with comment:19.
HL.Problems.DataAnalysis.Symbolic
- ~~ Delete empty and unused folder Transformation ~~ \ mkommend: Addressed with comment:20.
HeuristicLab.Problems.DataAnalysis Transformations
- Why do we need transformations?!?!
- Can't we delete all of them?
- If not, why don't we move them to the DataPreprocessing and forget about them?
gkronber: They are used in DataPreprocessing. After the changes in this ticket they are also used for scaling of input variables (instead of the Scaling class). Any symbolic regression model can have transformations. If we want to keep support for this at least the interface is necessary in the symbolic namespace.
Edit: After discussion we decided to move all transformations to the DataPreprocessing namespace and remove support for transformations from symbolic models.
Converters
I don't really get the benefit of the Convert class, besides listing somehow related conversion operations.\ gkronber: addressed with comment:22AFAIK the TreeSimplifier is stateless? If that's the case why don't we provide a static method for simplification. e.g.\ gkronber: addressed with comment:23var simplifiedTree = TreeSimplifier.Simplify(originalTree);
.TreeToAutoDiffTermConverter encapsulates the functionality nicely. This new refactoring would also enable to use exception for control flow handling in TryConvertToAutoDiff(node,term) instead of always checking the transformation result (true or false).\ gkronber: addressed with comment:24Finished reviewing the changes in this tickets.
Replying to [comment:18 mkommend]:
Enumerable Extensions
- Why has the ext. method TakeEvery been added, although it is never used? gkronber: Good question. I traced this back to r7154. Seems we never needed this. Can be removed.
r14944: Removed TakeEvery methods from EnumerableExtensions.cs
Replying to [comment:18 mkommend]:
HL.Problems.DataAnalysis.Symbolic
- Delete empty and unused folder Transformation r14945: Removed empty folder HL.Problems.DataAnalysis.Symbolic\Transformation.
Since MCTS has been removed (already merged to stable #2581) there are conflicts when merging this ticket to stable.
Issue migrated from trac ticket # 2697
milestone: HeuristicLab 3.3.15 | component: Problems.DataAnalysis | priority: medium | resolution: done
2016-11-09 09:43:49: @gkronber created the issue