Free cookie consent management tool by TermsFeed Policy Generator

Changes between Version 1 and Version 2 of Ticket #852, comment 44


Ignore:
Timestamp:
03/25/11 14:13:08 (14 years ago)
Author:
gkronber
Comment:

Legend:

Unmodified
Added
Removed
Modified

  • Ticket #852, comment 44

    v1 v2  
    11
    22Review comments:
    3  * as has been remarked already by swagner it is inconvenient that an exception is thrown immediately after a new problem is created (in all cases but real-valued test functions). This is caused by the fact that the !SwarmUpdater parameter in the PSO algorithm is a constrained-value parameter and thus does not allow null values. As a side remark the exception also occurs when a incompatible problem (which does not define a !SwarmUpdater operator) is dragged on the algorithm however in this case the exception is silently caught somewhere and the drag-and-drop operation shows no effect (this issue is probably not specific to PSO). It would be nice if the algorithm can handle such situations more gracefully similar to TS. In TS the situation is similar as it needs a !MoveGenerator operator defined in the problem. TS does not throw an exception when a problem without !MoveGenerators is created or opened.
     3 * as has been remarked already by swagner it is inconvenient that an exception is thrown immediately after a new problem is created (in all cases but real-valued test functions). This is caused by the fact that the `SwarmUpdater` parameter in the PSO algorithm is a constrained-value parameter and thus does not allow null values. As a side remark the exception also occurs when a incompatible problem (which does not define a `SwarmUpdater` operator) is dragged on the algorithm however in this case the exception is silently caught somewhere and the drag-and-drop operation shows no effect (this issue is probably not specific to PSO). It would be nice if the algorithm can handle such situations more gracefully similar to TS. In TS the situation is similar as it needs a `MoveGenerator` operator defined in the problem. TS does not throw an exception when a problem without `MoveGenerators` is created or opened.
    44
    5  * PSO has a number of !OptionalConstrainedValueParameters and !ConstrainedValueParameters for operators. The values of the parameters depend on each other and thus the selected value and valid values of dependent parameters are updated when the selected value of a parameter is changed by the user. However, the list of valid values in !ConstrainedValueParameters can be edited freely by the user via the GUI. It seems this case is ignored as the necessary parameter wiring is not present in the PSO class. This means it is possible for the user to set a combination of incompatible !TopologyUpdaters, !TopologyInitializers and this leads to an exception when the algorithm is executed. Either changing the valid values should be prevented by setting the parameter to readonly state or the wiring of !ConstrainedValueParameters should be improved.
     5 * PSO has a number of `OptionalConstrainedValueParameters` and `ConstrainedValueParameters` for operators. The values of the parameters depend on each other and thus the selected value and valid values of dependent parameters are updated when the selected value of a parameter is changed by the user. However, the list of valid values in `ConstrainedValueParameters` can be edited freely by the user via the GUI. It seems this case is ignored as the necessary parameter wiring is not present in the PSO class. This means it is possible for the user to set a combination of incompatible `TopologyUpdaters`, `TopologyInitializers` and this leads to an exception when the algorithm is executed. Either changing the valid values should be prevented by setting the parameter to readonly state or the wiring of `ConstrainedValueParameters` should be improved.
    66
    7  * In class PSO a !PlaceHolder operator for a velocity bounds updater is created but it seems this operator is never used
     7 * In class PSO a `PlaceHolder` operator for a velocity bounds updater is created but it seems this operator is never used.
    88
    9  * The class !VelocityBoundsUpdater (or modifier, the terminology is inconsitent here) seems ill-placed. As already suggested by abeham after an earlier review the functionality of the should be merged into a the !SwarmUpdater. Thus I would prefer to move the !VelocityBoundsUpdater class into the `Encodings.RealVector` plugin to the particle operators. I tried to set the !VelocityBoundsUpdater parameter in the `RealVectorSwarmUpdaterParameter`, but I was unable to set a configuration that can be executed. It seems this is impossible anyway as the statement in line 203 in class `RealVectorSwarmUpdater` tries to create a new operation for it's successor, but this will not succeed as the successor of `RealVectorSwarmUpdater` is null. It seems the velocity bounds updater is not really necessary for the PSO algorithm and maybe we should think about excluding this functionality from the next release.
     9 * The class `VelocityBoundsUpdater` (or modifier, the terminology is inconsitent here) seems ill-placed. As already suggested by abeham after an earlier review the functionality of the should be merged into a the `SwarmUpdater`. Thus I would prefer to move the `VelocityBoundsUpdater` class into the `Encodings.RealVector` plugin to the particle operators. I tried to set the `VelocityBoundsUpdater` parameter in the `RealVectorSwarmUpdaterParameter`, but I was unable to set a configuration that can be executed. It seems this is impossible anyway, as the statement in line 203 in class `RealVectorSwarmUpdater` tries to create a new operation for its successor, but this will not succeed as the successor of `RealVectorSwarmUpdater` is null. It seems the velocity bounds updater is not really necessary for the PSO algorithm and maybe we should think about excluding this functionality from the next release.
     10
     11 * the removal of the concept of a global best solution and instead handling this a special case of a global neighborhood seems to be a good idea. This should also be reflected in the PSO parameters. This means that it is probably a good idea to use a topology initializer and set it to 'Fully connected topology' as a default (I assume the terms neighborhood and topology are used interchangeably in the PSO implementation). I think this is already implemented as suggested.
     12
     13 * The separation between algorithm, problem, and encoding has been greatly improved, thanks!
     14
     15Concluding remarks: I think the PSO implementation is almost ready for release. The most important issues that should be addressed before the release are the fact that an exception is thrown when creating or opening any problem except for the 'real valued test functions problem' and the problem of the missing wiring of the `ConstrainedValueParameters`.