= [wiki:Reviews Reviews] - HeuristicLab 3.3.0 Application Review = Review comments which have been implemented are listed [wiki:ReviewHeuristicLab3.3.0ApplicationDone here]. == Reviewer: abeham == === Priority: HIGH === * The operator graph view is a bit confusing: * The successor parameter reads "Successor: !ResultsCollector", "Successor: !SubScopesSorter", ... It's more readable if "Successor" was not present all the time (in the operator graph group box). * swagner: "Successor" is required to identify an operator parameter. Of course, in the case of `ResultsCollector`, `SubScopesSorter`, etc. there is only one operator parameter, but there are other operators which have multiple operator parameters (e.g. branches). If we skip the parameter name, the user will not be able to tell which operator parameter is shown in the tree view. * The flow chart is very nice, but sometimes arrows are a bit wild and going backwards which creates a not so nice strikethrough appearance. Would be good to have non-direct arrows as well. * swagner: In a discussion with maffenze it was decided to address this issue after the release of HeuristicLab 3.3.0. * Adding a `ValueParameter` in a `VariableCreator` and selecting a generic type e.g. `ItemList` fails because no choice can be made on the generic type of the `ItemList` (ticket #42). * Even when a control is not displayed (e.g. the global scope tab) its view is updated which costs a lot of time. The optimization runs noticeably faster (~2x) when HeuristicLab isn't the active application in Windows (at least in Win7). === Priority: MEDIUM === * The TSP problem view: * Would be nice to have paste support (e.g. from Excel) in the datagrid of the string convertible matrix view. * Would be nice to be able to view a "scatter plot" of the coordinates. * `BestKnownTour` is missing as parameter. * All Parameters have the same icon, can we have different ones for values and operators? * The view host icon should act like a button that when single-clicked opens the context-menu, when hovering over it, there should be a frame around it like a button so that it's clear that it can be clicked. * swagner: When hovering over the view host icon a tool tip is displayed that explains how the different views can be selected. * In my user defined algorithm: * I have added an SGAOperator and set it as initial operator. I added crossover, mutation, evaluation and selection: `ProportionalSelector` has a Parameter "!CurrentScope" that cannot be changed in any way, is there a necessity to display this parameter? * swagner: The scope parameter "!CurrentScope" indicates that the current scope (i.e. its sub-scopes) is manipulated by an operator. It is shown in order to give the user comprehensive information about what an operator does. Maybe this is not really helpful. Should I remove scope parameters in general? In the code of an operator they are not really required, as the current scope can always be accessed via the current execution context. * swagner: In a discussion with maffenze it was decided to keep scope parameters. * `LeftSelector` and `RightSelector` should not be displayed as selection operators in an SGA (there is a related ticket #673). * When clicking "start" to start the engine disabling of the user interface elements has similar problems to what we fixed a few revisions ago (ticket #887) * In the operator graph chart view: * The vertical scroll bar allows scrolling way past the graph. * Long operator names require higher shapes as the operator name breaks into the next line. * Scrolling using the mouse wheel looks very odd, if this can't be fixed it better should be disabled. * Sometimes one of the ports (in port) remains selected even if clicked somewhere in the white space. * You can select a connector by drawing a selection rectangle that doesn't cover the connector, but covers the bounding rectangle of the connector. * !OpenView should be default double click behavior in operator graph chart. * Sometimes arrows cannot be selected (e.g. when they're pointing directly east). * Every `IDeepCloneable` item needs to have a public default constructor. If `Activator.CreateInstance` in `DeepCloneable.Clone` is given a second parameter "true", non-public constructors would also be used. * Viewing different result variables (switching between them) is impossible, because the results view is disabled when running the engine. * `TypeSelector` view: When only one single type is possible (e.g. `DoubleData`), select that type and auto-close with ok === Priority: LOW === * Focus is not removed from a textbox if one clicks anywhere in the "gray" area. * swagner: This is the behavior of Windows Forms. * There should be a nicer way of saying the chart controls are not installed instead of an exception. * swagner: The HeuristicLab setup will check if the Microsoft Chart Controls are installed and will show a meaningful error message. * Does it make sense to select no engine in an user defined algorithm? * The button for changing the evaluation in the TSP view features "+" which is used for adding, not setting in other views. * swagner: In a discussion with maffenze it was decided to use the "+" icon for adding and for setting values as long as there is no better idea which icon to use for setting values. * Does it make sense to change the name of result variables or to remove their values? * The quality chart looks great! * We should probably use the same colors we had in 1.1 for best/average/worst qualities. * "!BestKnownQuality" is not displayed currently. * Would be good to be able to hide certain lines. * At some point we'll certainly need export to png,eps functionality (would be great to be able to specify resolution and width/height). * The tooltips should not exceed a certain width. With a long description you've got one line running over the whole screen. * swagner: It is not possible to define a maximum text width and to enforce word wrapping for tool tips. Therefore line breaks have to be set for each tool tip text individually. Please specify which tool tips are too long so that appropriate line breaks can be added. * The `DataTableValuesCollector` says it can only display double values, why not ints? * I got a very cryptic "Operation is not valid due to the current state of the object" exception at one point building the TS operator (as discussed Parameters should have an optional attribute that says they should throw an error in case `ActualValue` is null after retrieving). * Going through the scopes with the arrow keys doesn't update the details view of the scope. * Resetting the engine doesn't update the global scope details view (user defined algorithm). * `TypeSelector` view: Focus should be in the search field when opening it the first time * `TypeSelector` view: Make double click select type and ok * `VariableCreator` throws an exception when injecting a variable whose value is null ---- == Reviewer: gkronber == === Priority: HIGH === * The plugin management GUI is not finished. === Priority: MEDIUM === * `HeuristicLab.Algorithms.TS` and `HeuristicLab.Algorithms.LS` should be renamed to `HeuristicLab.Algorithms.TabuSearch` and `HeuristicLab.Algorithms.LocalSearch`. * `HeuristicLab.Algorithms.SGA` should be renamed to `HeuristicLab.Algorithms.StandardGeneticAlgorithm` * Loading the "New Item" dialog takes a long time the first time the "New" action is activated. Probably an issue in the plugin infrastructure. * swagner: When opening the new dialog for the first time, all creatable items are instantiated which might take some time (although I do not really notice a long loading time of the new dialog on my machine). At the moment many items are shown in the new dialog for testing purposes (category "Test"). After these items have been removed, the loading time of the new dialog will be much faster. I will remove all `[Creatable("Test")]` attributes on items in the next version. * Breakpoints should be visually emphasized. * swagner: At the moment, breakpoints are marked red in the `OperatorTreeView` and will be visually emphasized in the `OperatorGraph` visualization in the next version. Where else should they be emphasized? * It seems strange now that operators have the property `Breakpoint`. It is more usual to set breakpoints on invocations/calls of operators. Also isn't the engine responsible for breakpoints? * swagner: As in HL 3.2 the `Breakpoint` property is implemented in operators and can be changed in the GUI of an `OperatorGraph`. Additionally, I decided to add a checkbox to each operator in order to be able to mark breakpoints there as well (this was not possible in HL 3.2). If an operator is marked as breakpoint, an engine stops its execution after this operator has been processed. I agree that it would be more intuitive to store it in some other location, which operators are currently breakpoints. However, I do not immediately know where. Any suggestions? * The execution state of an engine (e.g. Execution Time, Log...) is not persisted or restored correctly. * swagner: Fixed storing and loading of an engine's execution time and state in r2933. The log is not stored and loaded at the moment, as it is not persisted in an engine but is only generated and shown in the view. * Dropdown-boxes for crossover and mutation operator selection are empty. * swagner: They are empty as long as no problem has been selected as an algorithm cannot know a priori which crossover and mutation operators are allowed. === Priority: LOW === * `ProgrammableOperator`: 'Compile' is a normal button but 'Show Generated Code' looks like a label. I think the style should be unified. ---- == Reviewer: swinkler == === Priority: HIGH === * Adding a paramter to a `ProgrammableOperator`, for example a `ValueParameter`, leads to a null-pointer-exception. * (...) As a follow-up, after aborting adding a parameter to a `ProgrammableOperator`, a `PersistenceException` is thrown. === Priority: MEDIUM === * After starting a new SGA and loading a problem the "play" button is enabled; clicking it leads to a error report that might be not interpretable by each potential user. A rather complicated check of all parameters would be needed here (as we had it for example in HL 2) - this might be not essentially necessary for the initial release (?), but maybe we could keep it in mind. * swagner: Changed in r2924 that a selection and a crossover operator are automatically selected if available to avoid the cryptic error message. In general I would prefer to set reasonable parameters by default instead of implementing some (probably quite sophisticated) parameter validation procedure. === Priority: LOW === * Could it be reasonable to display a special icon for parameters that are not yet set properly (e.g., with a red exclamation mark etc.)? * swagner: This will be quite hard as it is not easy to decide programmatically, if a parameter is properly set or not. But we can think about it. Maybe we can introduce constrained parameters? Although it will be a lot of work to enable users to define constraints of parameters in the GUI (for example when adding a new parameter to a `CombinedOperator`). * On a screen with resolution 1280x800 I can for example not change the group size of tournament selection because the text box is not visible (unless the !OptionalValueParameter View is chosen). * swagner: Can you give me some hints where we can save space? Which information/controls might not be necessary? * swinkler: As names and data types cannot be edited in the details sections of parameters, I would suggest combining these two into one !TextBox and showing data types in brackets (as it is done in the list box shown left, e.g.). * Regarding the description of operators: * As the references to articles and books are incorporated in grammatically meaningful sentences (which I like a lot!) I would not use full stops (.) within references; I would use semicola (;) instead. * The name of an imported TSP instance is not stored / displayed. ---- == Reviewer: maffenze == === Priority: HIGH === * Something like a starter package of problems (which do not have to be imported before) should also be included in the release version. * swagner: We can include several problems in the setup as it was the case in HL 1.1 or we can offer a set of problems on the same web page where the HL setup can be downloaded. I would prefer the second solution, as it is simpler to realize and the user can easily choose where he wants to save the sample problems. Furthermore, he can download the sample problems again if necessary without having to reinstall HL. However, the first solution has the benefit that the user does not have to download two files. Is the second solution acceptable? * swagner: In a discussion with maffenze it was decided to take the second solution (i.e. to offer a set of problems on the HL download page). === Priority: MEDIUM === * After starting HeuristicLab it is not very obvious that the user has to choose the small new item button in order to have access to the standard methods. * swagner: Would it be an acceptable solution to show the new dialog automatically each time the HL 3.3 Optimizer is started? Another solution would be to show a wizard that guides the user through the first steps. If we want to have a wizard, should it show anything else than the available items that can be created? * gkronber: I think this is an important point because potential new users can be scared off within one minute if HeuristicLab doesn't have a few simple entry points to begin experimenting. One idea that comes to mind is to open a 'starter document' that has just 3 or 4 large buttons that activate a few basic entry points (e.g. "Genetic Algorithm: Optimize Tours", "Evolution Strategy: Find Minimum of Real-Valued Functions", "Show List of all Demos", "Load Algorithm From File"). Maybe we add an opt-out button for this starter screen. Sure a wizard of the kind "Solve your problem in 4 steps" would be nice but is probably not really necessary. * swagner: In a discussion with maffenze it was decided to show a "Start Page" when the optimizer is started that contains initial information how to use HeuristicLab and how to create new algorithms and problems. === Priority: LOW === ---- == Reviewer: vdorfer == First of all: I would regard a user manual with first steps (shall I first create a new TSP or create a new SGA and how can they be combined...) as very important, for me at the first sight it was not really clear how to start. === Priority: HIGH === === Priority: MEDIUM === * In the results view I would show by default the quality data table (instead of first having to stop (pause) the algorithm, select the data table and resume the algorithm). When the tour visualization is done, this would be best to show by default. * I would change the stop sign to a pause sign, as it actually pauses the run (maybe also change the tooltip of the start button to "Start/Resume Algorithm"). A stop sign signalizes for me: better not klick it, or else I have to start all over again * Add != Set Value; In all forms with a simple value (`IntData`, `DoubleData`, `BoolData`,..) a plus icon signalizes the possible change/edit. I would change the plus sign for example to a pencil (as you do not add a new value but change it). * A "Do you want to save your work" question is missing when clicking the "x" of an open SGA, TSP,... === Priority: LOW === * The comments in the generated code of a `ProgrammableOperator` are German. ---- == Reviewer: mkommend == === Priority: HIGH === === Priority: MEDIUM === * In my opinion a reordering of the tabs in the SGAView should be considered. The current order is Parameters, Problem, Engine, Results. It would be better if the order reflects the workflow to create an engine, e.g. Problem, Parameters, Results, Engine. === Priority: LOW === * The new dialog should also react on double clicks on the description column, if this is possible. ----