heal-research / HeuristicLab

HeuristicLab - An environment for heuristic and evolutionary optimization
https://dev.heuristiclab.com
GNU General Public License v3.0
39 stars 16 forks source link

ValueGenerator creates incorrect sequences due to the double arithmetic imprecision #2259

Closed HeuristicLab-Trac-Bot closed 9 years ago

HeuristicLab-Trac-Bot commented 10 years ago

Issue migrated from trac ticket # 2259

milestone: HeuristicLab 3.3.11 | component: Problems.Instances | priority: high | resolution: done

2014-10-08 13:08:01: @mkommend created the issue


Example: ValueGenerator.GeneratorSteps(0.05, 1, 0.05) produces: 0.05, 0.1, 0.15000000000000002, 0.2, 0.25, 0.3, 0.35, 0.39999999999999997, 0.44999999999999996, 0.49999999999999994, 0.54999999999999993, 0.6, 0.65, 0.70000000000000007, 0.75000000000000011, 0.80000000000000016, 0.8500000000000002, 0.90000000000000024, 0.95000000000000029, 1.0000000000000002 while the excpected result is: 0.05, 0.1, 0.15, 0.2, 0.25, 0.3, 0.35, 0.4, 0.45, 0.5, 0.55, 0.6, 0.65, 0.7, 0.75, 0.8, 0.85, 0.9, 0.95, 1.0

This could lead to incorrectly created problem instances, parameter ranges, etc.

HeuristicLab-Trac-Bot commented 10 years ago

2014-10-08 15:02:49: @abeham commented


Btw, we have similar logic in e.g. HeuristicLab.MainForm.WindowsForms.DefineArithmeticProgressionDialog (and the same problem). But notice that you can do it in 2 ways. Either start with the minimum and add the step (round it) until you reach or surpass the maximum. Or you could start from the maximum and reduce the step until you reach the minimum. The progression (1, 100, 10) would return 1, 11, 21, 31, ... in the first case and 1, 10, 20, ... in the second case. Maybe that was specific to the requirements of the CreateExperimentDialog where often the minimum of a parameter doesn't necessarily fit right into the progression see comment from swagner.

I think there should be some class in Common that computes such progresssions (and performs the necessary rounding) rather than distribute such ValueGenerators throughout the code.

HeuristicLab-Trac-Bot commented 10 years ago

2014-10-09 11:13:25: @mkommend commented


I agree that a common class for values generation should be used throughout HL. However, we created steps by starting from the start value and adding/subtracting the step width until the end is reached.

To achieve a sequence like you gave in your example (1,10,20, ...), which is not evenly spaced between the first two points you would have to call ValueGenerator.GenerateSteps(100,1,-10,includeEnd:true).Reverse().

A problem is that the ValueGenerator uses a HL.Random.FastRandom for creating of values according to a specified distribution(e.g., gaussian, uniform) and hence cannot be moved to HL.Common. An option would be to change the FastRandom to System.Random.

HeuristicLab-Trac-Bot commented 10 years ago

2014-10-09 11:16:24: @mkommend changed status from new to accepted

HeuristicLab-Trac-Bot commented 10 years ago

2014-10-09 11:16:24: @mkommend commented


r11434: Changed ValueGenerator to work with decimals instead of doubles to achieve a higher accuracy and adapted all problem instance providers accordingly.

HeuristicLab-Trac-Bot commented 10 years ago

2014-10-09 11:26:53: @mkommend commented


r11435: Added includeEnd parameter to ValueGenerator.GenerateSteps and removed overloads that take a Func<double,double> for value transformation.

HeuristicLab-Trac-Bot commented 10 years ago

2014-10-10 09:52:55: @mkommend commented


r11441: Changed expected values for training R² and neg log likelihood in Gaussian Process regression unit test.

The changes in the ValueGenerator resulted in two training rows to be slightly off (~E-16) for the generated spatial coevolution dataset and hence the unit test failed.

HeuristicLab-Trac-Bot commented 9 years ago

2014-10-13 13:29:11: @mkommend commented


The changes in this ticket depend on r11313 & r11348 from #2236.

HeuristicLab-Trac-Bot commented 9 years ago

2014-10-13 13:35:07: @mkommend commented


Ticket #2236 was closed as obsolete => r11313 & r11348 must be released with this ticket.

HeuristicLab-Trac-Bot commented 9 years ago

2014-10-21 14:54:16: @mkommend changed status from accepted to reviewing

HeuristicLab-Trac-Bot commented 9 years ago

2014-10-21 14:54:16: @mkommend changed owner from @mkommend to @abeham

HeuristicLab-Trac-Bot commented 9 years ago

2015-02-02 16:56:34: @abeham changed status from reviewing to readytorelease

HeuristicLab-Trac-Bot commented 9 years ago

2015-02-02 16:56:34: @abeham changed owner from @abeham to @mkommend

HeuristicLab-Trac-Bot commented 9 years ago

2015-02-02 16:56:34: @abeham commented


ok, changes look goood. I created #2301 to maybe put this class in a more prominent place. Or otherwise improve the numeric accuracy of the create experiment dialog.

HeuristicLab-Trac-Bot commented 9 years ago

2015-02-03 14:01:20: @mkommend changed status from readytorelease to closed

HeuristicLab-Trac-Bot commented 9 years ago

2015-02-03 14:01:20: @mkommend removed resolution

HeuristicLab-Trac-Bot commented 9 years ago

2015-02-03 14:01:20: @mkommend commented


r11868: Merged r11434, r11435, r11441 and r11313, r11348 into stable.