Opened 4 weeks ago

Last modified 44 hours ago

#2966 reviewing 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: branch
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 (22)

comment:1 Changed 4 weeks ago by chaider

  • Status changed from new to accepted

comment:2 Changed 4 weeks ago by chaider

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

comment:3 Changed 4 weeks ago by chaider

r16322: Added references to projects and added build events.

comment:4 Changed 4 weeks ago by chaider

r16323: Added Interval class and Interpreter class.

comment:5 Changed 4 weeks ago by chaider

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

r16324: Added test class to solution.

comment:6 Changed 4 weeks ago by mkommend

r16326: Corrected namespace of intervals.

comment:7 Changed 4 weeks ago by mkommend

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

comment:8 Changed 4 weeks ago by mkommend

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

comment:9 Changed 4 weeks 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 weeks ago by chaider

r16330: Implemented review comments and added IsCompatible method.

comment:11 Changed 3 weeks 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 11 days 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 11 days ago by mkommend

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

comment:14 Changed 8 days ago by chaider

r16363 Added GetVariableBoundaries method in DatasetUtil

comment:15 Changed 8 days 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 7 days ago by chaider

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

r16367 Added EvaluatedSolutions Parameter

comment:17 Changed 7 days ago by chaider

r16374 renamed variable from intermediate to result

comment:18 Changed 6 days ago by chaider

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

comment:19 Changed 6 days ago by chaider

r16377 Changed namespace to HeuristicLab.Problems.DataAnalysis.Symbolic

comment:20 Changed 6 days ago by chaider

r16380 added variable weight multiplication to the evaluate method

comment:21 Changed 5 days 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 44 hours ago by chaider

r16393 Checked null references

Note: See TracTickets for help on using tickets.