Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REVIEW]: pyGOURGS - global optimization of n-ary tree representable problems using uniform random global search #2074

Closed
whedon opened this issue Feb 5, 2020 · 68 comments

Comments

@whedon
Copy link
Collaborator

@whedon whedon commented Feb 5, 2020

Submitting author: @pySRURGS (Sohrab Towfighi)
Repository: https://github.com/pySRURGS/pyGOURGS
Version: 0.1
Editor: @arokem
Reviewer: @iljah, @nicoguaro
Archive: 10.5281/zenodo.3710475

Status

status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/17180193c5f25d3cc1edb460bc1335aa"><img src="https://joss.theoj.org/papers/17180193c5f25d3cc1edb460bc1335aa/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/17180193c5f25d3cc1edb460bc1335aa/status.svg)](https://joss.theoj.org/papers/17180193c5f25d3cc1edb460bc1335aa)

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

@iljah & @nicoguaro, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @arokem know.

Please try and complete your review in the next two weeks

Review checklist for @iljah

Conflict of interest

  • I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

General checks

  • Repository: Is the source code for this software available at the repository url?
  • License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • Contribution and authorship: Has the submitting author (@pySRURGS) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
  • Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

Review checklist for @nicoguaro

Conflict of interest

  • I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

General checks

  • Repository: Is the source code for this software available at the repository url?
  • License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • Contribution and authorship: Has the submitting author (@pySRURGS) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
  • Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?
@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Feb 5, 2020

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @iljah, @nicoguaro 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:

watching

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

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
@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Feb 5, 2020

Reference check summary:

OK DOIs

- 10.1186/s13040-018-0164-x is OK
- 10.1287/moor.6.1.19 is OK

MISSING DOIs

- https://doi.org/10.1016/0012-365x(71)90023-9 may be missing for title: Enumerating Trees
- https://doi.org/10.21105/joss.01675 may be missing for title: pySRURGS-a python package for symbolic regression by uniform random global search

INVALID DOIs

- None
@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Feb 5, 2020

@nicoguaro

This comment has been minimized.

Copy link

@nicoguaro nicoguaro commented Feb 5, 2020

I have installed the software as described. Nevertheless, I couldn't find an example of use in the documentation.

@pySRURGS

This comment has been minimized.

Copy link

@pySRURGS pySRURGS commented Feb 5, 2020

@nicoguaro , I will amend documentation to reflect suggested changes once I am back from interviews. There is an examples folder with script ant.py

@nicoguaro

This comment has been minimized.

Copy link

@nicoguaro nicoguaro commented Feb 5, 2020

I tried running the example with

$ python ant.py 

with output

Traceback (most recent call last):
  File "ant.py", line 118, in <module>
    output_db = args[0]
IndexError: list index out of range

with

$ python ant.py output

I get

Traceback (most recent call last):
  File "ant.py", line 119, in <module>
    n_iters = int(args[1])
IndexError: list index out of range

and, finally, with

$ python ant.py output 10

I didn't have any error message

@pySRURGS

This comment has been minimized.

Copy link

@pySRURGS pySRURGS commented Feb 5, 2020

The results would then be saved to the file called "output" without any extension.

@pySRURGS

This comment has been minimized.

Copy link

@pySRURGS pySRURGS commented Feb 5, 2020

There would have been 10 candidate solutions considered in that run.

@nicoguaro

This comment has been minimized.

Copy link

@nicoguaro nicoguaro commented Feb 5, 2020

Yes, that's what I understood by looking at the source code. But those things should be explained in the documentation.

@iljah

This comment has been minimized.

Copy link

@iljah iljah commented Feb 6, 2020

@pySRURGS please specify which university of medicine you are affiliated with.

@iljah

This comment has been minimized.

Copy link

@iljah iljah commented Feb 6, 2020

@pySRURGS it would be helpful if you'd mention how this relates to e.g. https://www.gnu.org/software/gsl/doc/html/multimin.html. Does this solve a more general problem or is this more efficient than GSL or..?

@iljah

This comment has been minimized.

Copy link

@iljah iljah commented Feb 6, 2020

As @nicoguaro mentioned, @pySRURGS please add at least one simple example to documentation. Also it would be helpful for example programs to print usage instructions when used incorrectly, etc.

@pySRURGS

This comment has been minimized.

Copy link

@pySRURGS pySRURGS commented Feb 6, 2020

University of Toronto.
It solves a different class of problems. It is more akin to traditional machine learning than it is to numerical optimization/function minimization. Thanks for the feedback and I will address these issues in coming days.

@pySRURGS

This comment has been minimized.

Copy link

@pySRURGS pySRURGS commented Feb 7, 2020

@iljah @nicoguaro
I have made the corrections you suggested. Please review the readme as it has been edited. Please review the ant.py example file as it has been greatly altered. It now has an interface that you can use from command line and this is discussed in the readme.

@nicoguaro

This comment has been minimized.

Copy link

@nicoguaro nicoguaro commented Feb 8, 2020

I guess that the line

$ winpty python ant.py -h

is for Windows (?).

I ran

$ python ant.py

and obtained

Traceback (most recent call last):
  File "ant.py", line 101, in <module>
    pset = pg.PrimitiveSet()
AttributeError: module 'pyGOURGS' has no attribute 'PrimitiveSet'
@pySRURGS

This comment has been minimized.

Copy link

@pySRURGS pySRURGS commented Feb 8, 2020

@nicoguaro
Yes winpty is for windows and has no effect on my code's function.
That is odd, and your error message is not that helpful. If the script is in the current directory and you are running it from terminal then I am not sure the cause of the error. I don't want to come across dismissive but did you git pull before running?

@pySRURGS

This comment has been minimized.

Copy link

@pySRURGS pySRURGS commented Feb 8, 2020

It is working fine on my end. If you are unable to run the code after git pulling and doing some sanity checks I can run on Linux later.

@pySRURGS

This comment has been minimized.

Copy link

@pySRURGS pySRURGS commented Feb 8, 2020

@nicoguaro I ran a fresh install of the software on linux and ran
python ant.py and did not encounter the issue.
please check that you ran the script from within the /examples/ directory, as I import the pyGOURGS package using a relative path.

@pySRURGS

This comment has been minimized.

Copy link

@pySRURGS pySRURGS commented Feb 8, 2020

I added commentary to the readme about running ant.py when in the /examples directory because of the relative path import of pyGOURGS

@nicoguaro

This comment has been minimized.

Copy link

@nicoguaro nicoguaro commented Feb 10, 2020

@nicoguaro I ran a fresh install of the software on linux and ran
python ant.py and did not encounter the issue.
please check that you ran the script from within the /examples/ directory, as I import the pyGOURGS package using a relative path.

I tried with a fresh install and it worked.

@nicoguaro nicoguaro closed this Feb 10, 2020
@nicoguaro nicoguaro reopened this Feb 10, 2020
@iljah

This comment has been minimized.

Copy link

@iljah iljah commented Feb 11, 2020

@pySRURGS thanks for making the changes, to me this looks good to go.

@pySRURGS

This comment has been minimized.

Copy link

@pySRURGS pySRURGS commented Feb 14, 2020

@nicoguaro I have added multiprocessing support to ant.py
You may need to run

pip install -r requirements.txt --user

while in the main directory in order to install new requirements.

@pySRURGS

This comment has been minimized.

Copy link

@pySRURGS pySRURGS commented Feb 18, 2020

@whedon generate pdf

@arokem

This comment has been minimized.

Copy link

@arokem arokem commented Mar 13, 2020

I see now. The DOI suggested by whedon is incorrect (points to another resource). I think that you can leave this as is.

In the meanwhile, while we are waiting for @nicoguaro's final approval, could you please go ahead and proceed with the following steps:

  • Make a tagged release of your software, and list the version tag of the archived version here.
  • Archive the reviewed software in Zenodo or a similar service (e.g. figshare, an institutional repository)
  • Check the archival deposit (e.g., in Zenodo) has the correct metadata, this includes the title (should match the paper title) and author list (make sure the list is correct and people who only made a small fix are not on it); you may also add the authors' ORCID.
  • Please list the DOI of the archived version here.

Once we have these items, and @nicoguaro's confirmation, we can move ahead with the acceptance of the paper.

@nicoguaro

This comment has been minimized.

Copy link

@nicoguaro nicoguaro commented Mar 13, 2020

@nicoguaro : I see that "example usage" is still unchecked. Just making sure: is this ready for publication from your point of view?

Yes, it is checked now.

@pySRURGS

This comment has been minimized.

Copy link

@pySRURGS pySRURGS commented Mar 13, 2020

All done

@arfon

This comment has been minimized.

Copy link
Member

@arfon arfon commented Mar 14, 2020

Dear authors and reviewers

We wanted to notify you that in light of the current COVID-19 pandemic, JOSS has decided to suspend submission of new manuscripts and to handle existing manuscripts (such as this one) on a "best efforts basis". We understand that you may need to attend to more pressing issues than completing a review or updating a repository in response to a review. If this is the case, a quick note indicating that you need to put a "pause" on your involvement with a review would be appreciated but is not required.

Thanks in advance for your understanding.

Arfon Smith, Editor in Chief, on behalf of the JOSS editorial team.

@arokem

This comment has been minimized.

Copy link

@arokem arokem commented Mar 19, 2020

@whedon set 10.5281/zenodo.3710475 as archive

@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Mar 19, 2020

OK. 10.5281/zenodo.3710475 is the archive.

@arokem

This comment has been minimized.

Copy link

@arokem arokem commented Mar 19, 2020

@whedon set 0.1 as version

@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Mar 19, 2020

OK. 0.1 is the version.

@arokem

This comment has been minimized.

Copy link

@arokem arokem commented Mar 19, 2020

@openjournals/joss-eics : this is ready for your review, when you get around to it.

@pySRURGS : please bear with us, as we do our best here.

@danielskatz

This comment has been minimized.

Copy link

@danielskatz danielskatz commented Mar 19, 2020

@whedon accept

@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Mar 19, 2020

Attempting dry run of processing paper acceptance...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Mar 19, 2020

Reference check summary:

OK DOIs

- 10.1109/4235.585893 is OK
- 10.1186/s13040-018-0164-x is OK
- 10.1287/moor.6.1.19 is OK
- 10.21105/joss.01675 is OK

MISSING DOIs

- https://doi.org/10.1016/0012-365x(71)90023-9 may be missing for title: Enumerating Trees

INVALID DOIs

- None
@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Mar 19, 2020

👋 @openjournals/joss-eics, this paper is ready to be accepted and published.

Check final proof 👉 openjournals/joss-papers#1381

If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#1381, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true
@danielskatz

This comment has been minimized.

Copy link

@danielskatz danielskatz commented Mar 19, 2020

👋 @pySRURGS - please merge pySRURGS/pyGOURGS#2 and pySRURGS/pyGOURGS#3 or let me know what you disagree with. These are changes based on proof-reading the final draft paper.

@pySRURGS

This comment has been minimized.

Copy link

@pySRURGS pySRURGS commented Mar 20, 2020

@danielskatz , I have merged all suggested changes.

@danielskatz

This comment has been minimized.

Copy link

@danielskatz danielskatz commented Mar 20, 2020

@whedon accept

@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Mar 20, 2020

Attempting dry run of processing paper acceptance...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Mar 20, 2020

Reference check summary:

OK DOIs

- 10.1109/4235.585893 is OK
- 10.1186/s13040-018-0164-x is OK
- 10.1287/moor.6.1.19 is OK
- 10.21105/joss.01675 is OK

MISSING DOIs

- https://doi.org/10.1016/0012-365x(71)90023-9 may be missing for title: Enumerating Trees

INVALID DOIs

- None
@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Mar 20, 2020

👋 @openjournals/joss-eics, this paper is ready to be accepted and published.

Check final proof 👉 openjournals/joss-papers#1384

If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#1384, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true
@danielskatz

This comment has been minimized.

Copy link

@danielskatz danielskatz commented Mar 20, 2020

@whedon accept deposit=true

@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Mar 20, 2020

Doing it live! Attempting automated processing of paper acceptance...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Mar 20, 2020

🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦

@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Mar 20, 2020

🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨

Here's what you must now do:

  1. Check final PDF and Crossref metadata that was deposited 👉 openjournals/joss-papers#1385
  2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.02074
  3. If everything looks good, then close this review issue.
  4. Party like you just published a paper! 🎉🌈🦄💃👻🤘

Any issues? notify your editorial technical team...

@danielskatz

This comment has been minimized.

Copy link

@danielskatz danielskatz commented Mar 20, 2020

Thanks to @iljah & @nicoguaro for reviewing, and to @arokem for editing!

And congratulations to @pySRURGS!

@whedon

This comment has been minimized.

Copy link
Collaborator Author

@whedon whedon commented Mar 20, 2020

🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](https://joss.theoj.org/papers/10.21105/joss.02074/status.svg)](https://doi.org/10.21105/joss.02074)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.02074">
  <img src="https://joss.theoj.org/papers/10.21105/joss.02074/status.svg" alt="DOI badge" >
</a>

reStructuredText:
.. image:: https://joss.theoj.org/papers/10.21105/joss.02074/status.svg
   :target: https://doi.org/10.21105/joss.02074

This is how it will look in your documentation:

DOI

We need your help!

Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
You can’t perform that action at this time.