= [wiki:Reviews Reviews] - HeuristicLab 3.3.0 Application Review = == Reviewer: abeham == I am not so happy with the priority separation, I'd rather give some impressions grouped by user actions / screens rather than by priorities. I like very much the SGA interface! === Priority: HIGH === * Creating a new TSP problem and viewing the parameter "Coordinates": The view can be switched to an "!OptionalValueParameterView" which adds a button to delete the value. However when clicked an exception is thrown that the value cannot be null. The !OptionalValueParameterView should not be selectable (Coordinates is not an optional parameter). This also holds for other parameters like the !SolutionCreator. * swagner: Changed in r2948. * When one creates a new user defined algorithm, the user finds a button called "Create User Defined Algorithm" button on the form. This feels strange. * swagner: Set the button to be invisible in r2947. * In the user defined algorithm: * On the parameter tab: I press "+" to add a new Parameter. I select a !ValueParameter, term it "!MutationRate" and select !DoubleData as type I get a !MissingMethodException (Constructor on type !ValueParameter not found). * swagner: Fixed in r2948. * 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. * Selecting an operator in "Operator Graph" does not bring up the details in the details view. * swagner: Fixed in r2949. * Should there really be a view host in the operator graph group box? It's confusing being able to switch views there (also because these are always just views on the initial operator which can be viewed with view host in the details group box as well). * swagner: Removed view host in r2947. * 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. * Creating a user defined algorithm from the SGA doesn't reconfigure the SGA operator with the parameters set in the SGA interface. === Priority: MEDIUM === * The TSP problem view: * How do I set the (allowed) evaluation operator? * swagner: Enabled changing values of value parameters in r2948. * Why do I want to change e.g. "TSPRoundedEuclideanPathEvaluator" in the textbox "Name" when viewing Parameter "Evaluator"? * swagner: It is always allowed to change the name of an operator in order to be able to use more descriptive names when building operator graphs. Of course, this feature is not meaningful for the operator parameters of the TSP. However, as it is a general feature of operators I cannot really remove it for the TSP operators. But maybe this feature is not required for all operators as only some operators (combined operators, placeholders, etc.) need to have a custom name? * 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 !ViewHost menu contains a lot of views which are quite similar. * !ItemView doesn't display anything, should it be selectable? * !ParameterView and !NamedItemView are very similar. * Sometimes there are only very fine differences between the views (e.g. a !ParameterView and a !NamedItemView). * 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. * 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. * After selecting !CurrentScope, all further parameters will be displayed as !ParameterView and appear as if they cannot be changed. I had to change the view manually in the view host. Sometimes it seems to select the "wrong" default views. It reverts back to !ParameterView as default as soon as !CurrentScope is clicked again. * swagner: Fixed in r2949. * !InversionManipulator has the "correct" actual name set already (Permutation), however in !OrderCrossover the default names are "Parents" and "Child" which have to be changed to Permutation. This isn't obvious right away when the string description of the parameter says: "Child: Child (Permutation)". * swagner: Set the actual name of the parents and child parameter to "Permutation" by default in r2947. * My initial expectations probably weren't correct regarding the user defined algorithm. The SGA operator is not a complete SGA, but just the main loop (yes we discussed this). * Left- and !RightSelector should not be displayed as selection operators in an SGA * When clicking "start" to start the engine disabling of the user interface elements has similar problems to what we fixed a few revisions ago. * I'd wish there was just a value view as well. Sometimes I want to hide name, description, type, set, remove, ... elements and just display the value (e.g. when viewing the quality chart) (we discussed this) === Priority: LOW === * Sequential engine should probably not be createable. At least currently there's nothing one could do with just an engine. * swagner: Removed `Creatable` attribute in r2924. * When creating e.g. a new TSP in the parameter collection view the tool tip doesn't really display the description of the parameter but just a very generic one. * swagner: Changed to more specific tool tips in r2947. * There is a typo in the description of the parameters (tooltips), it says "it" instead of "in". * swagner: Fixed in r2947. * "Breakpoint" doesn't explain if it breaks before or after executing the operator. * swagner: Added tool tip in r2949. * Focus is not removed from a textbox if one clicks anywhere in the "gray" area. * swagner: This is the behavior of Windows Forms. * The available parameters view lists just one namespace which is collapsed by default (should be expanded if it's just this one). * swagner: Changed in r2953. * 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 * When you remove the value of the results (such as !BestQuality) the text in the box does not update. * Does it make sense to remove these values? * Does it make sense to change the name of the variables under "Results"? * 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. ---- == Reviewer: gkronber == === Priority: HIGH === * The plugin management GUI is not finished. === Priority: MEDIUM === * 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. * Are keyboard shortcuts for groupboxes necessary? E.g. __D__etails groupbox and __D__escription shortcuts are conflicting. __V__alue groupbox and __V__alue textbox are conflicting. * swagner: Removed keyboard shortcuts for all groupboxes in r2924. * Description of !RandomCreator should contain a hint about the type of PRNG that is created. * swagner: Done in r2924. ---- == Reviewer: swinkler == === Priority: HIGH === * After starting an SGA the user is able to add a new problem; the OK button in the following dialog is initially enabled, clicking it causes no (?) action. * swagner: Fixed in r2924. * Adding a new parameter, for example a `ValueParameter`, leads to an exception. * swagner: Fixed in r2948. * After starting a SGA with 130 cities TSP and population size 100 the memory consumption is 1.2 GB after approx. 6500 generations. * swagner: Memory and runtime performance has been drastically improved in r2932. === 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: This issue is similar to one of the issues described by maffenze. 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. * After starting the SGA the current generation index is not displayed; I have to switch to the qualities chart to see the current generation. * swagner: Added generations counter to results in r2924. * During the run of an algorithm the values displayed for the variables `BestQuality`, `AverageQuality`, and `WorstQuality` flicker. * swagner: Fixed in r2924. === 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 (1): I would prefer to not to capitalize names of algorithms, i.e., for example I would prefer "... the standard genetic algorithm ..." over "... the Standard Genetic Algorithm ...". * Regarding the description of operators (2): 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 === * When no crossover is selected an error message is thrown which gives no hint about the reason. * swagner: This issue is similar to one of the issues described by swinkler. Changed in r2924 that a selection and a crossover operator are automatically selected if available to avoid the cryptic error message. * 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? === 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. === Priority: LOW === * It is not obvious what is meant with the breakpoint option in an operator. * swagner: Added tool tip in r2949. ----