Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @sjvrijn, @sarats it looks like you're currently assigned to review this paper 🎉.

⭐️ Important ⭐️

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

2. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

For a list of things I can do to help you, just type:

@whedon commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

👉 Check article proof 📄 👈

@gdetor Nice work. Having worked on a scalable parallel optimization framework myself, I agree with the motivation. Scaling island models is a challenging problem.

Figure 1 shows performance for various genome sizes. It is unclear if this uses MPI/threads. Please clarify.

As a performance specialist, I would have loved to see performance scaling plots for multiple MPI processes/threads to understand computational efficiency of the framework. This is not a binding requirement; just good-to-have information. Any data you can provide to demonstrate the parallel processing capability would be great.

@sarats Thank you for your comments. We will fix the issue you mentioned in Figure 1 and we will add some scaling plots.

@gdetor a few questions/remarks:

• install.sh seems to be missing (see https://gitlab.com/gdetor/genetic_alg/issues/2)

• I've tried to install the dependencies and run make, but even after installing libconfig++9v5 (as indicated in the dependency list) and libconfig-dev, I still get the following error message on Ubuntu 18.04.3 LTS:

Compiling src/selection.cpp
In file included from src/selection.cpp:26:0:
include/gaim.h:38:10: fatal error: libconfig.h++: No such file or directory
 #include <libconfig.h++>
          ^~~~~~~~~~~~~~~

• In the paper, you mention the potential use-cases of parameter optimization. Do you have some default configurations to recommend to potential users when they want to use it in this context? Otherwise they have to tune the options and parameters for GAIM instead, i.e. simply trading one hyperparameter optimization problem for another.

• For the configuration files, you give examples for each of the three sub-parts of the GA config section. Are those all possible options? If not, where can I find a total overview? If so, are any of them optional?

@sjvrijn Thank you for your comments/suggestions. We are going to add an installation file and address your comments. (bullets 3 and 4). For the time being the fix to your issue regarding libconfig is:
sudo apt install libconfig++9v5 libconfig++-dev

Let me know if you have any other issues regarding the installation of GAIM. The installation script will take care of all this in the future.

Best,
Georgios

@gdetor Thanks for adding the install.sh

Sadly, installing using install.sh fails on Ubuntu 16.04:

$ lsb_release -si && lsb_release -sr && ./install.sh
Ubuntu
16.04
========================================
Detecting the OS distro ...
========================================
========================================
Installing dependencies ... 
========================================
[sudo] password for rijnsjvan:            <-- Sudo password asked and accepted
Reading package lists... Done
Building dependency tree       
Reading state information... Done
E: Unable to locate package libopenmpi2   <-- libopenmpi1.10 is available and installed
========================================
Now compiling the standalone project ...
========================================
Cleaning objects
Cleaning binaries
Cleaning library
Compiling src/selection.cpp
In file included from src/selection.cpp:26:0:
include/gaim.h:38:25: fatal error: libconfig.h++: No such file or directory
compilation terminated.
Makefile:62: recipe for target 'build/selection.o' failed
make: *** [build/selection.o] Error 1

I can get further by manually installing the libconfig++ packages which installs some dependencies, although this leads to a bunch of new errors further on:

$ sudo apt-get install libconfig++9v5 libconfig++-dev 
Reading package lists... Done
Building dependency tree       
Reading state information... Done
The following additional packages will be installed:
  libconfig-dev libconfig-doc libconfig9
The following NEW packages will be installed:
  libconfig++-dev libconfig++9v5 libconfig-dev libconfig-doc libconfig9
[...]
$ ./install.sh 
[...]
Compiling src/island_ga.cpp
Linking bin/main
In file included from /usr/include/c++/5/random:35:0,
                 from include/gaim.h:31,
                 from gaim/main.cpp:1:
/usr/include/c++/5/bits/c++0x_warning.h:32:2: error: #error This file requires compiler and library support for the ISO C++ 2011 standard. This support must be enabled with the -std=c++11 or -std=gnu++11 compiler options.
 #error This file requires compiler and library support \
  ^
In file included from gaim/main.cpp:1:0:
include/gaim.h:104:26: warning: non-static data member initializers only available with -std=c++11 or -std=gnu++11
     bool is_im_enabled = false; /**< Boolean flag indicating if IM is enabled or not */
                          ^
include/gaim.h:241:38: error: ‘>>’ should be ‘> >’ within a nested template argument list
         std::map<int, std::vector<int>> adj_list;   /// Adjacent list of islands
                                      ^
include/gaim.h:314:6: error: ‘tuple’ in namespace ‘std’ does not name a template type
 std::tuple<ga_parameter_s, pr_parameter_s, im_parameter_s> read_parameters_file(const char *);
      ^
gaim/main.cpp: In function ‘int main()’:
gaim/main.cpp:4:5: error: ‘tuple’ is not a member of ‘std’
     std::tuple<ga_parameter_s, pr_parameter_s, im_parameter_s> pms;
     ^
gaim/main.cpp:4:30: error: expected primary-expression before ‘,’ token
     std::tuple<ga_parameter_s, pr_parameter_s, im_parameter_s> pms;
                              ^
gaim/main.cpp:4:46: error: expected primary-expression before ‘,’ token
     std::tuple<ga_parameter_s, pr_parameter_s, im_parameter_s> pms;
                                              ^
gaim/main.cpp:4:62: error: expected primary-expression before ‘>’ token
     std::tuple<ga_parameter_s, pr_parameter_s, im_parameter_s> pms;
                                                              ^
gaim/main.cpp:4:64: error: ‘pms’ was not declared in this scope
     std::tuple<ga_parameter_s, pr_parameter_s, im_parameter_s> pms;
                                                                ^
gaim/main.cpp:9:5: error: ‘tie’ is not a member of ‘std’
     std::tie(ga_pms, pr_pms, im_pms) = read_parameters_file("./examples/demo.cfg");
     ^
gaim/main.cpp:9:82: error: ‘read_parameters_file’ was not declared in this scope
     std::tie(ga_pms, pr_pms, im_pms) = read_parameters_file("./examples/demo.cfg");
                                                                                  ^
Makefile:49: recipe for target 'bin/main' failed
make: *** [bin/main] Error 1

My g++ version:

$ g++ --version
g++ (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609

@gdetor I also think there is a typo in install.sh:

rpms="g++ make openmpi libconfig libcongig-devel libconfig++-devel
                                       ^

@sjvrijn Regarding the linking errors you're getting. Try to replace the -std=c++0x instead of -std=c++11 in the Makefile (line 4) and let me know if that worked. The problem is essentially your gcc version (kind of old). Thank you also for spotting the typo in install.sh (fixed).

@gdetor I've tested again on a fresh install of Ubuntu 19.10. With a newer version (9.2.1) the g++ compiler is indeed no longer a problem, but the libconfig++ dependencies are still not installed properly by the install.sh script.

After running sudo apt install libconfig++9v5 libconfig++-dev manually, it does compile and work as intended.

Here is the console output of my install: (I had already used sudo apt install ... earlier in this same terminal session)

$ git clone https://gitlab.com/gdetor/genetic_alg.git
Cloning into 'genetic_alg'...
remote: Enumerating objects: 467, done.
remote: Counting objects: 100% (467/467), done.
remote: Compressing objects: 100% (210/210), done.
remote: Total 1294 (delta 287), reused 421 (delta 248)
Receiving objects: 100% (1294/1294), 1.41 MiB | 2.43 MiB/s, done.
Resolving deltas: 100% (693/693), done.

$ cd genetic_alg/

$ ./install.sh 
========================================
Detecting the OS distro ...
========================================
========================================
Installing dependencies ... 
========================================
Reading package lists... Done
Building dependency tree       
Reading state information... Done
E: Unable to locate package libopenmpi2
========================================
Now compiling the standalone project ...
========================================
Cleaning objects
Cleaning binaries
Cleaning library
Compiling src/selection.cpp
In file included from src/selection.cpp:26:
include/gaim.h:38:10: fatal error: libconfig.h++: No such file or directory
   38 | #include <libconfig.h++>
      |          ^~~~~~~~~~~~~~~
compilation terminated.
make: *** [Makefile:63: build/selection.o] Error 1

$ sudo apt install libconfig++9v5 libconfig++-dev 
Reading package lists... Done
Building dependency tree       
Reading state information... Done
The following additional packages will be installed:
  libconfig-dev libconfig-doc libconfig9
The following NEW packages will be installed:
  libconfig++-dev libconfig++9v5 libconfig-dev libconfig-doc libconfig9
0 upgraded, 5 newly installed, 0 to remove and 3 not upgraded.
[... details omitted for brevity ...]

$ ./install.sh 
========================================
Detecting the OS distro ...
========================================
[... details omitted for brevity ...]
Linking bin/main

$

@gdetor Now I' ve been able to test it, I have a couple more remarks:

• I would recommend changing print_best_genome = false; to true in examples/demo.cfg, as it now leads to an error when running the plot_results.py script:
  FileNotFoundError: [Errno 2] No such file or directory: './data/best_genome_0.dat'
• Am I correct that the bounds a and b as given in the configuration file are only used for initialization? For example, if I invert the sphere function, the best found solution using demo.cfg is (164.7, 193.0), well outside the bounds of [-1, 1]
• main_mpi_island_model.cpp is missing from the file-list in the 'More examples' section of the Readme
• The mpi_island example also points to the regular demo.cfg file. Should that be demo_island_model.cfg instead?
• Can you add calling examples for the mpi cases? I'm not too experienced with it and ran into some errors with the mpi_island:

Using demo_island_model.cfg and updated error message:

$ ./mpi_island 
Simulation: GA_demo_simulation

Number of Islands (3) mistmatches MPI rank size (1)!

$ mpirun -n 3 ./mpi_island 
make: 'mpi_island' is up to date.
Simulation: GA_demo_simulation

Simulation: GA_demo_simulation

Simulation: GA_demo_simulation

[host:13745] *** An error occurred in MPI_Dist_graph_create_adjacent invalid sources
[host:13745] *** reported by process [3954638849,2]
[host:13745] *** on communicator MPI_COMM_WORLD
[host:13745] *** MPI_ERR_ARG: invalid argument of some other kind
[host:13745] *** MPI_ERRORS_ARE_FATAL (processes in this communicator will now abort,
[host:13745] ***    and potentially your MPI job)

The independent runs version worked fine:

$ ./mpi_independent_runs 
Simulation: GA_demo_simulation

Writing the average fitness to a file.
Writing the BSF fitness to a file.

$ mpirun -n 3 ./mpi_independent_runs 
Simulation: GA_demo_simulation

Simulation: GA_demo_simulation

Simulation: GA_demo_simulation

Writing the average fitness to a file.
Writing the BSF fitness to a file.
Writing the average fitness to a file.
Writing the BSF fitness to a file.
Writing the average fitness to a file.
Writing the BSF fitness to a file.

@sjvrijn

• We changed the print_best_genome to true by default.
• The intervals are indeed used in the initialization process, however they can also be used by some mutation operators to keep genomes within a specific range.
• We added the main_mpi_island_model.cpp in the file-list in the More Examples section
• We kept the same configuration file to show that the user needs only one config file to operate the GAIM. We could create a new one with the name you suggest.
• We can add some examples for the MPI cases as well.
• The error you get when you run the mpi_island is related to the number of MPI processes you specify at the runtime. The number of MPI processes should match the number of islands specified in the xxxxx_graph.dat file. In the present case, you should use #mpirun -n 5 mpi_island.

We haven't yet pushed the abovementioned changes since we are still dealing with all reviewers' comments. We will let you once we push the new code.

@sjvrijn In reference to your previous comments that our last response did not yet address:

1. No, we don't have recommended hyperparameter settings because we feel there aren't general purpose recommendations that everybody can agree upon -- as much as that would constitute an ideal state of affairs. Appropriate settings would be somewhat dependent on the objective and the computing budget available to the researcher.

2. The only settings that have choices that are specifically named are
   candidate_immigrants_pickup_method and candidate_immigrants_replace_method.
   The README file has been updated to clarify there are three values for each of
   those settings (random, elit, poor) and explains a little about each of those settings. Other settings have somewhat intuitive bounds or are filepaths. There is more detail in the documentation. It also has been changed to reflect the fact that unused settings are not actually to be removed from the configuration file. Irrelevant settings will be ignored (in only that sense are they optional), and there are no further settings included at present that are absent in the examples (i.e. all the settings currently there are in the example).

3. We added more details about manual installation and that the installation instructions pertain only to the installation of the dependencies. The examples section is introduced a little differently to emphasize the fact that the full example including the skeleton code is given at the end of that section, and that is where the library is compiled in the tutorial information. We recommend the manual installation because it is difficult to create a fully-automated installation script that covers all the available versions of even just the most common Linux distros.

@gdetor Thanks for the thorough updates and replies 👍 I have now been able to install and run all examples without any issues.

A final reply regarding the hyperparameter recommendations: I can completely understand your reason there, so I guess the demo configurations will have to suffice for anyone without too much knowledge about (island model) GA's 🙂

@majensen This concludes the review from my part.

@sjvrijn thanks very much - @sarats -- have your performance concerns been addressed (I see that this box is not yet checked in your review ) ?

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

PDF failed to compile for issue #1839 with the following error:

/app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:377:in parse': (tmp/1839/paper.md): did not find expected alphabetic or numeric character while scanning an alias at line 18 column 11 (Psych::SyntaxError) from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:377:in parse_stream'
from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:325:in parse' from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:252:in load'
from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:473:in block in load_file' from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:472:in open'
from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:472:in load_file' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-df8b50fe58b8/lib/whedon.rb:125:in load_yaml'
from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-df8b50fe58b8/lib/whedon.rb:85:in initialize' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-df8b50fe58b8/lib/whedon/processor.rb:36:in new'
from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-df8b50fe58b8/lib/whedon/processor.rb:36:in set_paper' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-df8b50fe58b8/bin/whedon:55:in prepare'
from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/command.rb:27:in run' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/invocation.rb:126:in invoke_command'
from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor.rb:387:in dispatch' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/base.rb:466:in start'
from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-df8b50fe58b8/bin/whedon:116:in <top (required)>' from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in load'
from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in `

@sarats We fixed the problem with the current figure and we added three more figures indicating the performance for both the Island Model using POSIX threads and MPI processes. We tested different population sizes and different number of threads/proccesses.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

👉 Check article proof 📄 👈

Thanks @gdetor for including detailed performance analysis.
Just a couple of minor suggestions below.

• The x-axis label for sub-figures B and C is better off as Number of Threads/Tasks for clarity.
• In sub-figure B: It's a bit surprising to see that much overhead with OpenMPI vs. POSIX threads, perhaps the MPI implementation needs tuning.

@majensen This is good to go from my end.

@sarats Thank you for the comments. The problem was that the optimization flags were disabled for the MPI. We are going to correct the figure.

@majensen Thank you for the editing. I just merged your edits.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

👉 Check article proof 📄 👈

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

👉 Check article proof 📄 👈

Hi @gdetor - There's a little typo in my merge, a space between [Schwefel...] and (..link..) (see screenshot). I made a merge request, but it's probably easier if you could go in and delete that space. Once that is achieved, can I ask you to create a new tag from the master, and then archive that tag with [Zenodo][https://zenodo.org/] or similar service? Then please provide the archive DOI in this thread. I will then make push the recommendation to the editors-in-chief.

Thanks!

Hi, @majensen I corrected the typo. Thanks for spotting that. Do you need any specific tag on the master branch? Do you have any specific instructions for archiving on Zenodo? Thank you in advance.

@gdetor - don't need a specific tag, but just a persistent marker to the version of the software that was reviewed and recommended. Re: Zenodo, it's pretty easy - create an account at https://zenodo.org/ , you can use GitHub or ORCID to log in. You upload a tarball of your repo (ideally) at the tag.
Here's a quick run through: at the upload screen, select the tar.gz (or what have you) file from your local machine to upload, and you will get the following form:

You can fill it out with as much detail as you want - but make sure you specify an appropriate open source license. Zenodo will provide a free DOI for the archive -

Please go ahead and report that DOI back to this thread. This will be included in the paper and you can use it as a convenient pointer to your software (the DOI for the paper itself is different and will be provided by JOSS)

You can get a badge with the DOI to use wherever:

HTH!

Hi, @majensen The reserved Zenodo DOI is: 10.5281/zenodo.3558829
The tarball is uploaded on Zenodo as well.

@whedon set 10.5281/zenodo.3558829 as archive

OK. 10.5281/zenodo.3558829 is the archive.

@whedon set v1.0.0 as version

OK. v1.0.0 is the version.

@openjournals/joss-eics this paper is recommended for publication and ready for final check - thanks!

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

PDF failed to compile for issue #1839 with the following error:

Error producing PDF.
! Argument of \caption@ydblarg has an extra }.

\par
l.473 test function.}

Looks like we failed to compile the PDF

@gdetor uhoh - there is a problem with pdf compile that must be due to that link for the Schwefel fn. I should have checked the proof before. Could you have a look-- maybe we can simply include the url in parens without making it a hyperlink.
Apologies for that

Edit- I have updated the paper in my fork to remove the hyperlink in the markdown and just put the url in parens. This compiles fine for me locally, and is clickable in the resulting pdf. There is an open merge request that should include this change -- the commit is at https://gitlab.com/majensen1/genetic_alg/commit/aed1cec31a2ce88bde1b95a68b94390d17849f93

@openjournals/joss-eics please stand by until the compile issue in #1839 (comment) is fixed - thanks

Hi, @majensen The link has been fixed as you indicated. It should be fine now.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

👉 Check article proof 📄 👈

@openjournals/joss-eics, proof issues have been resolved. Please begin final check for publication. Thanks!

👋 @gdetor - I will shortly generate some PRs for the paper and bib - please merge them or let me know what you don't agree with...

bib PR: https://gitlab.com/gdetor/genetic_alg/merge_requests/3

paper PR: https://gitlab.com/gdetor/genetic_alg/merge_requests/4