Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign up[REVIEW]: GAIM: A C++ library for Genetic Algorithms and Island Models #1839
Comments
This comment has been minimized.
This comment has been minimized.
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 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:
For a list of things I can do to help you, just type:
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
|
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@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. |
This comment has been minimized.
This comment has been minimized.
@sarats Thank you for your comments. We will fix the issue you mentioned in Figure 1 and we will add some scaling plots. |
This comment has been minimized.
This comment has been minimized.
@gdetor a few questions/remarks:
|
This comment has been minimized.
This comment has been minimized.
@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: 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, |
This comment has been minimized.
This comment has been minimized.
@gdetor Thanks for adding the install.sh Sadly, installing using install.sh fails on Ubuntu 16.04:
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:
My
|
This comment has been minimized.
This comment has been minimized.
@gdetor I also think there is a typo in
|
This comment has been minimized.
This comment has been minimized.
@sjvrijn Regarding the linking errors you're getting. Try to replace the |
This comment has been minimized.
This comment has been minimized.
@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 After running Here is the console output of my install: (I had already used
|
This comment has been minimized.
This comment has been minimized.
@gdetor Now I' ve been able to test it, I have a couple more remarks:
Using
The independent runs version worked fine:
|
This comment has been minimized.
This comment has been minimized.
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. |
This comment has been minimized.
This comment has been minimized.
@sjvrijn In reference to your previous comments that our last response did not yet address:
|
This comment has been minimized.
This comment has been minimized.
@gdetor Thanks for the thorough updates and replies 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. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@whedon generate pdf |
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
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 |
This comment has been minimized.
This comment has been minimized.
@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. |
This comment has been minimized.
This comment has been minimized.
@whedon generate pdf |
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Thanks @gdetor for including detailed performance analysis.
@majensen This is good to go from my end. |
This comment has been minimized.
This comment has been minimized.
@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. |
This comment has been minimized.
This comment has been minimized.
@majensen Thank you for the editing. I just merged your edits. |
This comment has been minimized.
This comment has been minimized.
@whedon generate pdf |
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@whedon generate pdf |
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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! |
This comment has been minimized.
This comment has been minimized.
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. |
This comment has been minimized.
This comment has been minimized.
@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. 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! |
This comment has been minimized.
This comment has been minimized.
Hi, @majensen The reserved Zenodo DOI is: 10.5281/zenodo.3558829 |
This comment has been minimized.
This comment has been minimized.
@whedon set 10.5281/zenodo.3558829 as archive |
This comment has been minimized.
This comment has been minimized.
OK. 10.5281/zenodo.3558829 is the archive. |
This comment has been minimized.
This comment has been minimized.
@whedon set v1.0.0 as version |
This comment has been minimized.
This comment has been minimized.
OK. v1.0.0 is the version. |
This comment has been minimized.
This comment has been minimized.
@openjournals/joss-eics this paper is recommended for publication and ready for final check - thanks! |
This comment has been minimized.
This comment has been minimized.
@whedon generate pdf |
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
PDF failed to compile for issue #1839 with the following error: Error producing PDF. Looks like we failed to compile the PDF |
This comment has been minimized.
This comment has been minimized.
@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. 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 |
This comment has been minimized.
This comment has been minimized.
@openjournals/joss-eics please stand by until the compile issue in #1839 (comment) is fixed - thanks |
This comment has been minimized.
This comment has been minimized.
Hi, @majensen The link has been fixed as you indicated. It should be fine now. |
This comment has been minimized.
This comment has been minimized.
@whedon generate pdf |
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@openjournals/joss-eics, proof issues have been resolved. Please begin final check for publication. Thanks! |
This comment has been minimized.
This comment has been minimized.
|
whedon commentedOct 28, 2019
•
edited
Submitting author: @gdetor (Georgios Detorakis)
Repository: https://gitlab.com/gdetor/genetic_alg
Version: v1.0.0
Editor: @majensen
Reviewer: @sjvrijn, @sarats
Archive: 10.5281/zenodo.3558829
Status
Status badge code:
Reviewers and authors:
Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)
Reviewer instructions & questions
@sjvrijn & @sarats, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @majensen know.
Review checklist for @sjvrijn
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
Review checklist for @sarats
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper