Opened 3 months ago

Last modified 2 months ago

#2966 readytorelease feature request

Implement Intervals and according calculation rules

Reported by: mkommend Owned by: mkommend
Priority: medium Milestone: HeuristicLab 3.3.16
Component: Problems.DataAnalysis Version: trunk
Keywords: Cc:

Description

This ticket collects the changes for the implementation of intervals and calculation rules and acts as base for a-priori knowledge integration and further model analysis. The following class will be added in this ticket:

  • Interval
  • Interpreter for symbolic models to calculate intervals of a model
  • Unit test for the calculation with intervals

Change History (29)

comment:1 Changed 3 months ago by chaider

  • Status changed from new to accepted

comment:2 Changed 3 months ago by chaider

r16320: Created Branch with problems data analysis and problems data analysis symbolic.

comment:3 Changed 3 months ago by chaider

r16322: Added references to projects and added build events.

comment:4 Changed 3 months ago by chaider

r16323: Added Interval class and Interpreter class.

comment:5 Changed 3 months ago by chaider

  • Owner changed from chaider to mkommend
  • Status changed from accepted to reviewing

r16324: Added test class to solution.

comment:6 Changed 3 months ago by mkommend

r16326: Corrected namespace of intervals.

comment:7 Changed 3 months ago by mkommend

r16327: Changed bounds to auto property in Interval and corrected typo in comment.

comment:8 Changed 3 months ago by mkommend

r16328: Reordered methods in interval interpreter, added IStatefulItem interface, sealed class.

comment:9 Changed 3 months ago by mkommend

  • Owner changed from mkommend to chaider
  • Status changed from reviewing to assigned

Review comments

Interval

  • Everything looks file

IntervalInterpreter

  • Source file is missing an I
  • Item Description must be improved. It reads currently "Interpreter for interval arithmetic within symbolic regression.", which is only partially true, because this is an interpreter for calculation of intervals of arbitrary symbolic models (e.g., it could be also used for linear regression).
  • Line 46: What is the purpose of GetSymbolicExpressionTreeIntervals(tree, rows, out intervals)? How can this method calculate anything if no ranges for inputs or a dataset is provided?
  • Why does each method for interval calculation has a row parameter? What is the purpose of the parameter? Is it really beneficial to calculate an interval of a dataset only for parts of it?
  • Why is there no method GetSymbolicExpressionTreeInterval without an out parameter?
  • What is the purpose of the syncRoot if it is not used at all?
  • Line 77: move declaration of unmodified varialbe necessaryArgStackSize closer to its usage in Line 112. Perhaps define it as a const.
  • Is it not possible to get rid of the interpreter state and just use an array of instructions?
  • Refactor assignment conditionals for interval calculation
    • if(customIntervals[varName] != null ...
  • Implement null check for dataset and customIntervals as preconditions at the start of an method
  • Check for variable occurrences once to avoid the containskey statements every time
  • Evaluate should have the intervals parameter as optional parameter and only if it is assigned the intermediate intervals should be calculated

Feel free to ask questions if anything is unclear.

comment:10 Changed 3 months ago by chaider

r16330: Implemented review comments and added IsCompatible method.

comment:11 Changed 3 months ago by chaider

  • Owner changed from chaider to mkommend
  • Status changed from assigned to reviewing

r16331: Changed Evaluate method (changed intervals parameter to optional)

comment:12 Changed 2 months ago by mkommend

Review Comments

IntervalInterpreter

  • Line 142 throws an Exception instead of an ArgumentException
  • Line 144 fails if the Dataset is null, which is not checked
  • Line 152 checks for custom intervals however no error handling is implemented if the specific custom interval is not present (line 153).
  • Extraction of intervals for variables of the dataset should be done only once; in contrast to each occurrence of the variable.

comment:13 Changed 2 months ago by mkommend

  • Owner changed from mkommend to chaider
  • Status changed from reviewing to assigned

comment:14 Changed 2 months ago by chaider

r16363 Added GetVariableBoundaries method in DatasetUtil

comment:15 Changed 2 months ago by chaider

r16364

  • Changed signature of GetSymbolicExressionTreeIntervals methods
  • Changed PrepareInterpreterState (removed optinal parameters, takes Dictionary<string, Interval> as input and no dataset anymore)
  • Added optional parameter (rows) to GetVariableBoundaries method (allows to use just parts from dataset as input e.g. training/test indices)

comment:16 Changed 2 months ago by chaider

  • Owner changed from chaider to mkommend
  • Status changed from assigned to reviewing

r16367 Added EvaluatedSolutions Parameter

comment:17 Changed 2 months ago by chaider

r16374 renamed variable from intermediate to result

comment:18 Changed 2 months ago by chaider

r16376 Removed member variable InstructionCount and added ref parameter to Evaluate method instead

comment:19 Changed 2 months ago by chaider

r16377 Changed namespace to HeuristicLab.Problems.DataAnalysis.Symbolic

comment:20 Changed 2 months ago by chaider

r16380 added variable weight multiplication to the evaluate method

comment:21 Changed 2 months ago by chaider

r16383

  • Some renaming and reordering
  • Changed GetVariableRanges method in DatasetUtil
  • Added cases for substract and divide with arity 1

comment:22 Changed 2 months ago by chaider

r16393 Checked null references

comment:23 Changed 2 months ago by chaider

r16403 Changed variable names

comment:24 Changed 2 months ago by chaider

r16404

  • Variable name changes
  • Changed GetVaribleRanges in DatasetUtil
  • Added method to get interval ranges from a double enumeration in Interval class
  • Added some comments

comment:25 Changed 2 months ago by chaider

r16406 Changed isEqual method in Interval

comment:26 Changed 2 months ago by chaider

r16407: Merged branch changes into trunk.

comment:27 Changed 2 months ago by chaider

  • Status changed from reviewing to readytorelease

r16409: deleted branch

comment:28 Changed 2 months ago by mkommend

  • Version changed from branch to trunk

comment:29 Changed 2 months ago by mkommend

r16436: Merged r16407 into stable.

Note: See TracTickets for help on using tickets.