Opened 2 years ago

Closed 2 years ago

#2259 closed defect (done)

ValueGenerator creates incorrect sequences due to the double arithmetic imprecision

Reported by: mkommend Owned by: mkommend
Priority: high Milestone: HeuristicLab 3.3.11
Component: Problems.Instances Version: 3.3.10
Keywords: Cc:


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.

Change History (10)

comment:1 Changed 2 years ago by abeham

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.

comment:2 Changed 2 years ago by mkommend

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.

comment:3 Changed 2 years ago by mkommend

  • Status changed from new to accepted

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

comment:4 Changed 2 years ago by mkommend

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

comment:5 Changed 2 years ago by mkommend

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.

comment:6 Changed 2 years ago by mkommend

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

comment:7 Changed 2 years ago by mkommend

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

comment:8 Changed 2 years ago by mkommend

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

comment:9 Changed 2 years ago by abeham

  • Owner changed from abeham to mkommend
  • Status changed from reviewing to readytorelease

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.

comment:10 Changed 2 years ago by mkommend

  • Resolution set to done
  • Status changed from readytorelease to closed

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

Note: See TracTickets for help on using tickets.