Reviews - HeuristicLab 3.3.0 Application Review

Review comments which have been implemented are listed here.

Reviewer: abeham

Priority: HIGH

• The operator graph view is a bit confusing:
  • 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<T> in a VariableCreator and selecting a generic type e.g. ItemList<T> fails because no choice can be made on the generic type of the ItemList (ticket #42).
• The OperatorGraphVisualization doesn't respect operator collections (e.g. SequentialSubScopesProcessor).
  • mkommend: This feature will rarely be used by a standard user and it will not be fixed right now. Although special shapes for MultipleCallOperators are planned.
• In TypeSelector filter types that don't have a constructor (like currently PathTSPTour).
  • swagner: Generally all items should provide a parameterless constructor. I implemented one for PathTSPTour in r3139.
• When the TSP importer is used after halting the execution, the engine should be reset.
• PermutationTypes is missing a view
• When there is an error in one of the item's constructors, the TypeSelector dialog (when pressing the "new" button) will open the first time, but stop listing items after encountering the item with the error. The second time the TypeSelector is shown through the "new" button, the optimizer crashes with an exception.
• A KeyNotFoundException is thrown in Values_ItemsAdded when a DataTableView is closed while it is updated.

Priority: MEDIUM

• Saving a problem doesn't remember the filename when saving again.
• The TSP problem view:
  • Would be nice to have paste support (e.g. from Excel) in the datagrid of the string convertible matrix view.
• In the operator graph chart view:
  • Some boxes are sometimes placed above the top end of the chart.
    • mkommend: This behavior could not be reproduced in r3176.
  • Top-down layouting would be better instead of left-right for two reasons:
    1. The boxes are usually much wider than high and thus a left-right layout makes the graph even longer.
    2. A top-down graph better fits the paper orientation in a publication.
    • mkommend: This will be implemented with the other changes regarding the layout of the operator graph.
• 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.
  • swagner: See ticket #922.
• TypeSelector view: When only one single type is possible (e.g. DoubleData), select that type and auto-close with OK.
  • In a discussion with maffenze it was decided to show the type selector dialog in every case as we fear that it might be irritating for a user, if the dialog is sometimes shown and sometimes not shown.
• A problem should inject a variable that indicates its dimensionality (sometimes you want to set values depending on the dimension of the problem).
• The description in the start page should show the respective icon or menu entry as crop out of a screenshot in the explanation of what to do.
  • swagner: Added some button images in r3225.
• The default action for drag & drop actions should be copy (e.g. to/from the clipboard). Linking is more advanced, assumes programming knowledge, and can lead to all sorts of unwanted situations.
• Adding a new parameter through the GUI and choosing a ConstrainedValueParameter does not make sense as the set of valid values cannot be defined anywhere in the GUI. The parameter will never be able to take a value. Additionally, switching the view for this ConstrainedValueParameter to a ValueParameterView and setting a value there leads to an exception. The ValueParameterView should not be a view for the ConstrainedValueParameter.
• Adding a new ValueParameter<T> could create a default value if T is instantiable. For most ValueParameters you will want to have a value upon creation.

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.
  • abeham: I mentioned this because I want to validate the control by clicking out of the text box and thus losing the focus. Maybe this can be achieved by allowing focus on the control or form. I think by default the "gray area" doesn't accept focus.
• 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.
• The quality chart looks great!
  • We should probably use the same colors we had in 1.1 for best/average/worst qualities.
  • Would be good to be able to hide certain lines.
  • At some point we'll certainly need export to png, eps, etc. 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).
• VariableCreator throws an exception when injecting a variable whose value is null.
• 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.
  • abeham: I'm still not fully happy with the view host icon, but it's probably the least priority thing right now
• Some operators have 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.
  • abeham: I think more parameter are just more confusing and description of the internal mechanisms of an operator are better displayed in the description text. I'll vote for removal of the current scope parameters, but I'll move this to low priority for now.
• Improve description of SearchIntervalFactor in BreederGeneticAlgorithmManipulator.
• EvolutionStrategy:
  • Add parameter to ES to indicate if 1/5 success rule or sigma-self adapation should be used.
• Name in docking tab is not synchronized with the item name it displays.
• We do not currently count the number of evaluated solutions.

---

Reviewer: gkronber

Priority: HIGH

• The plugin management GUI is not finished.
• In the global scope view: If the selected item in the tree view is changed via keyboard (cursor keys) the Variables view is not updated.
• Closing the Optimizer while the global scope view is shown (user defined algorithm) and expanded throws ArgumentException in HeuristicLab.Core.Views.ScopeView.Dispose().
• Exception in BestQualityMemorizer because the actual name of the quality parameter is not updated. (To reproduce open a new user defined GA and then load a problem that changes the actual name of the quality parameter and then start the engine.)

Priority: MEDIUM

• 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?

Priority: LOW

---

Reviewer: swinkler

Priority: HIGH

• After loading a TSP instance into a TS and letting the TS, saving the TSP instance takes very long time (~ ten seconds)
  • abeham: It seems that the execution context and scope tree are persisted together with the problem. This is not specific to the TS, but with the exhaustive move generator the scope tree is noticeably larger and thus takes more time.
  • As a follow-up, loading this saved TSP instance fails (PersistenceException).
    • abeham: Fixed in r3206.
• After creating a new TSP instance the user has to define the coordinates matrix; setting the number of columns to 2 and the rows to 4, e.g., leads to an overflow error.
• Using a population with 0 individuals leads to an overflow exception, no matter which selector is chosen.
• Charts (results display, quality progress, ...) can slow down the execution very much; it would be nice to have a parameter that sets the update interval.

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.
• I would suggest displaying the name of an item in the tab head; e.g., "Evolution Strategy" instead of "Item3.hl"
• There is no way to see the optimal solution that is found by the algorithm, for example the best tour or the best knapsack selection. Even though not all users might be interested to see these best results, it might help to increase the confidence of users.

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.
• After changing the number of weights in a Knapsack problem there is an exception explaining that the number of weights is not equal to the number of values. This is quite understandable, but it could be better to give a parameter that defines the number of items and let the numbers of values and weights be set automatically.

---

Reviewer: maffenze

Priority: HIGH

Priority: MEDIUM

Priority: LOW

---

Reviewer: vdorfer

Priority: HIGH

• It is possible to start an algorithm with a TSP without having loaded the TSPLib file --> leads to IndexOutOfRangeException.

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.
  • swagner: Showing results by default is critical, as the continuous redraw of a result's value (e.g. quality chart, tour visualization) might consume a lot of runtime. Therefore the user should have to select a result manually, if he wants to look at a result.
• A "Do you want to save your work" question is missing when clicking the "x" of an open SGA, TSP, ...
• A TabuSearch with a ch150 TSP is not running fluently, it jerks.
  • swagner: Which parameter settings of the tabu search were used? If an exhaustive move generator is used, the tabu search jerks as computing the whole set of possible moves is quite time-consuming.
  • vdorfer: I used the default parameter settings, which have an ExhaustiveTwoOptMoveGenerator as move generator.

Priority: LOW

• The comments in the generated code of a ProgrammableOperator are German.
  • epitzer: This is a feature of the .NET runtime and depends on the installed operating system language.

---

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

---

Reviewer: epitzer

Priority: HIGH

• Searching for "prog" in the operator pane and using backspace twice => application hangs.
  • abeham: See also ticket #904.
• Empty TSP problem always throws an exception
• If later a problem is loaded, the algorithm still throws an exception (maybe automatically reset algorithm?).
• A Help menu should be available, maybe also an about dialog.

Priority: MEDIUM

• Some algorithm parameters cannot be set until problem is selected: display reason why parameters cannot be changed yet or maybe make the Problem tab the default one for Algorithms.
• Problem parameters that cannot be changed by the user could be hidden.
  • i.e. Maximation is always the first parameter but can rarely be changed.
  • swagner: In a discussion with maffenze it was decided not to hide parameters. The parameters collection of a problem should show all parameters of a problem so that a user sees and understands how the underlying problem object looks like. This should help users when they start to implement own problems.
• The [+] Button in the ValueParameterView should also be disabled if the type of the parameter cannot be changed.
  • swagner: Even if the type of a value parameter cannot be changed, it might be necessary to be able to exchange the whole value object. Therefore the [+] button should be enabled in every case.
• New Item Dialog: Algorithm and Problem items should be listed before the advanced items in "Algorithm Design".
  • swagner: In a discussion with maffenze it was decided to keep alphabetical sorting of creatable groups in the new item dialog. Alphabetical sorting is used in many places throughout the whole application and it might be irritating for a user if we change this in the new item dialog. Furthermore, it depends on a user which creatable groups are considered to be more important.
• Parameters of sub-operators that are not changeable by the user should be hidden (e.g. lookup parameter).
• Graph View
  • Icon for expanding/collapsing details is rather confusing (undo/redo), should be +/- imho
  • A zoom rectangle tool would be nice
  • panning while pressing the middle mouse button would be nice too
  • display updates while scrolling and panning are rather slow

Priority: LOW

• Import from TSPLIB button is easily overlooked (TSP Problem).
• Show results tab when engine is started, set a default item that is selected too (e.g. quality chart).
  • swagner: Showing results by default is critical, as the continuous redraw of a result's value (e.g. a quality chart) might consume a lot of runtime. Therefore the user should have to select a result manually, if he wants to look at a result.
• Hierarchical grouping of operators by namespace in operator tab (instead of just by plugin) or maybe just remove the intial "HeuristicLab" as this is common for all of them and makes it harder to scan for the right plugin/namespace.
  • swagner: In a discussion with maffenze it was decided not to change the way how operators are grouped and displayed in the operators sidebar. Reasons are that grouping hierarchically by namespaces might lead to deep trees that have to be opened to get an operator and that the initial "HeuristicLab" in each plugin name might not be common for all plugins if an external user develops an own plugin.

---