Opened 3 years ago

Closed 3 years ago

#2171 closed task (done)

Integrate ScilabParameterOptimizationProblem in the trunk

Reported by: mkommend Owned by: mkommend
Priority: medium Milestone: HeuristicLab 3.3.10
Component: Problems.ExternalEvaluation Version: 3.3.9
Keywords: Cc:

Description

The functionality to optimize real-vector encoded problems, whose solutions are evaluated by Scilab should be integrated in the trunk. The implementation was done in ticket #2082 and this ticket tracks the commits necessary for the trunk integration.

Change History (21)

comment:1 Changed 3 years ago by mkommend

  • Status changed from new to accepted

comment:2 Changed 3 years ago by mkommend

r10601: Added folder in ExtLibs for the Scilab DotNetComponent.

comment:3 Changed 3 years ago by mkommend

r10602: Added DotNetScilab and HeuristicLab.DotNetScilab to the external libraries.

comment:4 Changed 3 years ago by mkommend

r10603: Updated ExtLibs solution.

comment:5 Changed 3 years ago by mkommend

r10604: Updated plugin file for DotNetScilab.

comment:6 Changed 3 years ago by mkommend

r10605: Added new ParameterOptimizationProblem and the external evaluation with Scilab to the trunk.

comment:7 Changed 3 years ago by mkommend

r10606: Added build dependencies to HeuristicLab project.

comment:8 Changed 3 years ago by mkommend

r10607: Corrected output directories of Problems.ParameterOptimization and Problems.ExternalEvaluation.Scilab.

comment:9 Changed 3 years ago by mkommend

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

comment:10 Changed 3 years ago by abeham

r10653: corrected build configuration for x86 and x64

comment:11 Changed 3 years ago by gkronber

Last edited 3 years ago by gkronber (previous) (diff)

comment:12 Changed 3 years ago by gkronber

r11075: added reference to DotNetScilab-1.0 to the HL wrapper plugin

comment:13 follow-ups: Changed 3 years ago by gkronber

  • Owner changed from gkronber to mkommend

Review comments:

  • In the ScilabParameterVectorEvaluator access to the scilab connector is synchronized because "it is not possible to parallelize evaluation". I think the lock object must also be static since it should be used to synchronize access to the static ScilabConnector instance. (please review the change in r11076)
  • In line 89 in ScilabParameterVectorEvaluator the check if initializationScript is null or empty is not necessary because this is checked above. (please review the change in r11076)
  • if the value returned by Scilab is NaN or infinity the quality is set to double.MaxValue. However, if this is a maximization problem this is incorrect. (please review the change in r11076)
  • Why are the parameters of the strategyVectorManipulator changed when the evaluator is changed? They only depend on the ProblemSize, so I propose to update these parameters whenever the ProblemSize changes (please review the change in r11077).
  • It seems strange that the problem has a 'BestSolutionAnalyzer' and a 'BestSolutionsAnalyzer'. I think we can remove the 'BestSolutionAnalyzer'.
  • The GetHashCode method in the DoubleArrayEqualityComparer casts doubles to int thereby losing all digits after the comma. This could be problematic if e.g. all parameter values must lie in the interval [0..1[. The proposed change uses BitConverter to get a long representation of the double and calculates the hash value based on this representation (please review the change in r11077).
  • The BestSolutionsAnalyer relies on the fact that another analyzer updates the BestQuality correctly. The BestQuality parameter is only set initially to an extremly bad value when it is null. (In contrast the BestSolutionAnalyzer sets the parameter every time a better solution is found). I think this should be implemented in the same way in the BestSolutionsAnalyzer.
  • The set of solutions is cleared if the new solutions are better than previousBestQuality. I think the set should be cleared when the new solutions are better than 'BestQuality' (related to the previous comment).

comment:14 in reply to: ↑ 13 Changed 3 years ago by mkommend

  • Status changed from reviewing to assigned

Reviewed r11076.

  • In the ScilabParameterVectorEvaluator access to the scilab connector is synchronized because "it is not possible to parallelize evaluation". I think the lock object must also be static since it should be used to synchronize access to the static ScilabConnector instance. (please review the change in r11076)

Thanks for correcting this.

  • In line 89 in ScilabParameterVectorEvaluator the check if initializationScript is null or empty is not necessary because this is checked above. (please review the change in r11076)

This is not the case, because the previous check verifies only if the file exists when the file value is not null or empty. This doesn't mean that the optional initializationScript is set to a value.

  • if the value returned by Scilab is NaN or infinity the quality is set to double.MaxValue. However, if this is a maximization problem this is incorrect. (please review the change in r11076)

Yes, I forgot about the maximization case.

comment:15 Changed 3 years ago by mkommend

  • Status changed from assigned to accepted

comment:16 Changed 3 years ago by mkommend

r11080: Readded string.IsNullOrEmpty check for the initialization script in the ScilabParameterVectorEvaluator.

comment:17 in reply to: ↑ 13 Changed 3 years ago by mkommend

r11081: Updated best quality in the scilab solutions analyzer.

comment:18 Changed 3 years ago by mkommend

Reviewed r11077 and implemented the suggested changes from gkronber with r11080 and r11081.

comment:19 Changed 3 years ago by mkommend

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

Please review r11080 and r11081. If these changes are OK for you, the functionality of this ticket can be released.

comment:20 Changed 3 years ago by gkronber

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

Reviewed r11080 and r11081. Everything seems fine.

comment:21 Changed 3 years ago by mkommend

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

r11151: Merged r10601:r10607, r10653, r11075, r11076, r11077, r11080, r11081 into stable.

Note: See TracTickets for help on using tickets.