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]: SIMSOPT: A flexible framework for stellarator optimization #3525

Closed
40 tasks done
whedon opened this issue Jul 22, 2021 · 62 comments
Closed
40 tasks done

[REVIEW]: SIMSOPT: A flexible framework for stellarator optimization #3525

whedon opened this issue Jul 22, 2021 · 62 comments
Assignees
Labels
accepted CMake published Papers published in JOSS Python recommend-accept Papers recommended for acceptance in JOSS. review Shell

Comments

@whedon
Copy link

whedon commented Jul 22, 2021

Submitting author: @landreman (Matt Landreman)
Repository: https://github.com/hiddenSymmetries/simsopt
Version: v0.4.7
Editor: @dfm
Reviewer: @ZedThree, @StanczakDominik
Archive: 10.5281/zenodo.5498111

⚠️ JOSS reduced service mode ⚠️

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

status

Status badge code:

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

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

@ZedThree & @StanczakDominik, 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 @dfm know.

Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest

Review checklist for @ZedThree

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 (@landreman) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

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: Does the paper have a section titled 'Statement of Need' that clearly states 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 @StanczakDominik

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 (@landreman) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

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: Does the paper have a section titled 'Statement of Need' that clearly states 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
Copy link
Author

whedon commented Jul 22, 2021

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

⚠️ JOSS reduced service mode ⚠️

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

⭐ 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

@dfm
Copy link

dfm commented Jul 22, 2021

@ZedThree, @StanczakDominik, @landreman – This is the review thread for the paper. All of our communications will happen here from now on. Thanks again for agreeing to participate!

Please read the "Reviewer instructions & questions" in the first comment above.

Both reviewers have checklists at the top of this thread (in that first comment) with the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. There are also links to the JOSS reviewer guidelines.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention openjournals/joss-reviews#3525 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for the review process to be completed within about 4-6 weeks but please make a start well ahead of this as JOSS reviews are by their nature iterative and any early feedback you may be able to provide to the author will be very helpful in meeting this schedule.

@dfm
Copy link

dfm commented Jul 22, 2021

@whedon generate pdf from branch joss_paper

☝️ Since the paper lives on the joss_paper branch in the repo, I think we'll need to use this command to compile the paper.

@whedon
Copy link
Author

whedon commented Jul 22, 2021

Attempting PDF compilation from custom branch joss_paper. Reticulating splines etc...

@danielskatz
Copy link

@whedon commands

@whedon
Copy link
Author

whedon commented Jul 22, 2021

Here are some things you can ask me to do:

# List all of Whedon's capabilities
@whedon commands

# Assign a GitHub user as the sole reviewer of this submission
@whedon assign @username as reviewer

# Add a GitHub user to the reviewers of this submission
@whedon add @username as reviewer

# Re-invite a reviewer (if they can't update checklists)
@whedon re-invite @username as reviewer

# Remove a GitHub user from the reviewers of this submission
@whedon remove @username as reviewer

# List of editor GitHub usernames
@whedon list editors

# List of reviewers together with programming language preferences and domain expertise
@whedon list reviewers

# Change editorial assignment
@whedon assign @username as editor

# Set the software archive DOI at the top of the issue e.g.
@whedon set 10.0000/zenodo.00000 as archive

# Set the software version at the top of the issue e.g.
@whedon set v1.0.1 as version

# Open the review issue
@whedon start review

EDITORIAL TASKS

# All commands can be run on a non-default branch, to do this pass a custom 
# branch name by following the command with `from branch custom-branch-name`.
# For example:

# Compile the paper
@whedon generate pdf

# Compile the paper from alternative branch
@whedon generate pdf from branch custom-branch-name

# Remind an author or reviewer to return to a review after a
# certain period of time (supported units days and weeks)
@whedon remind @reviewer in 2 weeks

# Ask Whedon to do a dry run of accepting the paper and depositing with Crossref
@whedon recommend-accept

# Ask Whedon to check the references for missing DOIs
@whedon check references

# Ask Whedon to check repository statistics for the submitted software
@whedon check repository

EiC TASKS

# Invite an editor to edit a submission (sending them an email)
@whedon invite @editor as editor

# Reject a paper
@whedon reject

# Withdraw a paper
@whedon withdraw

# Ask Whedon to actually accept the paper and deposit with Crossref
@whedon accept deposit=true

@danielskatz
Copy link

@whedon generate pdf from branch joss_paper

@whedon
Copy link
Author

whedon commented Jul 22, 2021

Attempting PDF compilation from custom branch joss_paper. Reticulating splines etc...

@danielskatz
Copy link

@whedon check references from branch joss_paper

@whedon
Copy link
Author

whedon commented Jul 22, 2021

Attempting to check references... from custom branch joss_paper

@whedon
Copy link
Author

whedon commented Jul 22, 2021

Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.27 s (840.7 files/s, 131387.4 lines/s)
--------------------------------------------------------------------------------
Language                      files          blank        comment           code
--------------------------------------------------------------------------------
Python                          114           3886           5991          14065
C++                              22            292             86           3000
C/C++ Header                     32            458            192           2847
reStructuredText                 21            510            407           1211
YAML                             11            188            190            761
SparForte                         5             12              0            629
Markdown                          5             59              1            172
Bourne Again Shell                9             49             82             88
INI                               1             10              0             81
CMake                             1             15             12             76
DOS Batch                         1              8              1             26
CSS                               2              2             12             24
make                              1              5              7             12
Bourne Shell                      1              0              2              3
TOML                              1              0              0              3
--------------------------------------------------------------------------------
SUM:                            227           5494           6983          22998
--------------------------------------------------------------------------------


Statistical information for the repository '95ef2adc3d80f9a1544fb250' was
gathered on 2021/07/22.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Andrew Giuliani                 84          6965           4537           11.80
Bharat Kumar Medasan             8           117            286            0.41
Bharat Medasani                128         13491           2650           16.56
Caoxiang Zhu                     9           281             40            0.33
Elizabeth                       25          2267            397            2.73
Florian Wechsung               256         24779          12397           38.14
Matt Landreman                 153         14899           8198           23.70
Rogerio Jorge                   70          3440           1672            5.24
Zhisong Qu                       1             5              3            0.01
ejpaul                           5           649            402            1.08

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
Andrew Giuliani            2332           33.5          3.0               13.16
Bharat Medasani            6167           45.7          3.8               17.42
Elizabeth                   954           42.1          1.6                7.13
Florian Wechsung          11254           45.4          2.8                4.70
Matt Landreman             7849           52.7          5.8               18.21
Rogerio Jorge              2262           65.8          2.2                9.37
Zhisong Qu                    2           40.0          6.1                0.00

@whedon
Copy link
Author

whedon commented Jul 22, 2021

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

 Can't find any papers to compile :-(

@arfon
Copy link
Member

arfon commented Jul 22, 2021

@whedon generate pdf from branch joss_paper

@whedon
Copy link
Author

whedon commented Jul 22, 2021

Attempting PDF compilation from custom branch joss_paper. Reticulating splines etc...

@whedon
Copy link
Author

whedon commented Jul 22, 2021

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@dfm
Copy link

dfm commented Jul 22, 2021

@ZedThree, @StanczakDominik, @landreman — sorry about all the noise! Our bot @whedon was having issues, but it looks like we're back up and running.

@StanczakDominik
Copy link

Hey, I just wanted to report back and say that I'm sorry for the delay. I've had an intense few weeks. I'm now heading for my first vacation in a couple years (gee thanks, COVID!). Once I'm back next Tuesday, I'm making going through the rest of the package my first priority. The parts I've seen so far are looking great, by the way!

@whedon
Copy link
Author

whedon commented Aug 5, 2021

👋 @StanczakDominik, please update us on how your review is going (this is an automated reminder).

@whedon
Copy link
Author

whedon commented Aug 5, 2021

👋 @ZedThree, please update us on how your review is going (this is an automated reminder).

@ZedThree
Copy link

ZedThree commented Aug 6, 2021

Mostly done, I need to dig a bit more into the functionality and API docs. But generally, everything's been of a high standard so far.

@StanczakDominik
Copy link

Mostly done with my review! Everything looks, well, stellar. 🌞


One concern I have is the inclusion/dependency on SPEC. Based on some of my digging around (and hiddenSymmetries/simsopt#35 (comment) ), my understanding is that:

  • SIMSOPT wraps SPEC.
  • SPEC itself is closed source.
  • The provided Docker image contains compiled SPEC binaries, which SIMSOPT can use.
  • The creation of those binaries is carried out during the Docker image build, using keys to the private repo as Github secrets.
  • This means that I have been unable to run the SPEC-dependent examples locally, only through Docker.
  • This, in turn, means I've been unable to actually visualize the 3D results wherever the plotting is written using Mayavi, due to X11/docker weirdness (see Mayavi vs Pyvista, from the docker standpoint hiddenSymmetries/simsopt#134 for a future-work proposal to sidestep the issue).

I have a few ideas about this, in order of increasing feasibility:

  1. Open source SPEC! This is of course both the ideal world scenario and out of your hands, and was apparently in-progress-but-will-take-a-while in April: Docker container containing SIMSOPT + SPEC + VMEC hiddenSymmetries/simsopt#35 (comment) ;
  2. Contribute the SPEC binary building process of the SIMSOPT docker build upstream, so SPEC can remain closed source but provide the binaries (probably a waste of time depending on how close SPEC is to becoming open source, though);
  3. Note more visibly (in the paper? in the SIMSOPT readme? maybe both?) that, as of {date}, SPEC is on the way to open source, but not there yet, and for now people can use the Docker image that provides its binaries. This kind of thing should be stated openly, just so people don't have to dig into this issue themselves.
    • Ideally, add some notes on the 3d viz out of Docker issue, which I'm not sure has been considered (I imagine SIMSOPT devs have SPEC access!).

Besides that, though, really a cool piece of software and I definitely hope I'll get to use it some day :)

@landreman
Copy link

@StanczakDominik Thanks for taking the time to look at SIMSOPT. We definitely recognize the issue about SPEC not being open-source yet, and have been working on it. SPEC was developed at a national lab so there is a lot of bureaucracy to go through for this. We checked in again today with the people at the lab, and all the paperwork has been submitted to US DOE to make SPEC open-source, but we have to wait for their final approval to make the repo public, and I don't know how long this will take. In the meantime we followed your option 3: I added text to the simsopt README, to the index page of the docs, and to the JOSS paper to say that we expect SPEC to be open-source soon but it's not there yet. We also appreciate the point about mayavi. We'll work to allow other rendering packages for those plot functions, either replacing mayavi or providing other choices.

@landreman
Copy link

@whedon generate pdf from branch joss_paper

@whedon
Copy link
Author

whedon commented Sep 9, 2021

Attempting PDF compilation from custom branch joss_paper. Reticulating splines etc...

@whedon
Copy link
Author

whedon commented Sep 9, 2021

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@mbkumar
Copy link

mbkumar commented Sep 9, 2021

@dfm
version number incremented to v0.4.7
DOI is 10.5281/zenodo.5498111
https://dx.doi.org/10.5281/zenodo.5498111

@dfm
Copy link

dfm commented Sep 9, 2021

@whedon set v0.4.7 as version

@whedon
Copy link
Author

whedon commented Sep 9, 2021

OK. v0.4.7 is the version.

@dfm
Copy link

dfm commented Sep 9, 2021

@whedon set 10.5281/zenodo.5498111 as archive

@whedon
Copy link
Author

whedon commented Sep 9, 2021

OK. 10.5281/zenodo.5498111 is the archive.

@dfm
Copy link

dfm commented Sep 9, 2021

@whedon recommend-accept

@whedon
Copy link
Author

whedon commented Sep 9, 2021

Attempting dry run of processing paper acceptance...

@whedon whedon added the recommend-accept Papers recommended for acceptance in JOSS. label Sep 9, 2021
@whedon
Copy link
Author

whedon commented Sep 9, 2021

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

 Can't find any papers to compile :-(

@dfm
Copy link

dfm commented Sep 9, 2021

@whedon recommend-accept from branch joss_paper

@whedon
Copy link
Author

whedon commented Sep 9, 2021

Attempting dry run of processing paper acceptance...

@whedon
Copy link
Author

whedon commented Sep 9, 2021

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1063/1.864116 is OK
- 10.1063/1.4765691 is OK
- 10.1088/1361-6587/abc08e is OK
- 10.1088/0741-3335/59/1/014018 is OK
- 10.13182/fst95-a11947086 is OK
- 10.1016/0010-4655(86)90109-8 is OK
- 10.1063/1.524170 is OK
- 10.1063/1.864692 is OK
- 10.1088/1741-4326/aa8e0a is OK
- 10.1016/0375-9601(88)90080-1 is OK
- 10.1038/s41592-019-0686-2 is OK
- 10.1088/1741-4326/aaed50 is OK
- 10.1103/physrevlett.80.528 is OK
- 10.1063/1.872844 is OK
- 10.1088/1361-648x/ab82d2 is OK
- 10.1063/5.0061665 is OK

MISSING DOIs

- None

INVALID DOIs

- None

@whedon
Copy link
Author

whedon commented Sep 9, 2021

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

Check final proof 👉 openjournals/joss-papers#2574

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

@whedon accept deposit=true from branch joss_paper 

@dfm
Copy link

dfm commented Sep 9, 2021

@ZedThree, @StanczakDominik — Thanks for your reviews of this submission! I really appreciate the time that you volunteered.

@landreman — I have now passed this off to the Editors-in-Charge who may have some final edits before processing the acceptance step. But, from me, thanks for your submission and your work responding to the reviews!

@arfon
Copy link
Member

arfon commented Sep 10, 2021

@whedon accept deposit=true from branch joss_paper

@whedon whedon added accepted published Papers published in JOSS labels Sep 10, 2021
@whedon
Copy link
Author

whedon commented Sep 10, 2021

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

@whedon
Copy link
Author

whedon commented Sep 10, 2021

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

@whedon
Copy link
Author

whedon commented Sep 10, 2021

🚨🚨🚨 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 👉 Creating pull request for 10.21105.joss.03525 joss-papers#2578
  2. Wait a couple of minutes, then verify that the paper DOI resolves https://doi.org/10.21105/joss.03525
  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...

@arfon
Copy link
Member

arfon commented Sep 10, 2021

@ZedThree, @StanczakDominik – many thanks for your reviews here and to @dfm for editing this submission! JOSS relies upon the volunteer effort of people like you and we simply wouldn't be able to do this without you ✨

@landreman – your paper is now accepted and published in JOSS ⚡🚀💥

@arfon arfon closed this as completed Sep 10, 2021
@whedon
Copy link
Author

whedon commented Sep 10, 2021

🎉🎉🎉 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.03525/status.svg)](https://doi.org/10.21105/joss.03525)

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

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

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:

@landreman
Copy link

@ZedThree, @StanczakDominik, and @dfm: thank you for taking the time to review and edit our paper!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted CMake published Papers published in JOSS Python recommend-accept Papers recommended for acceptance in JOSS. review Shell
Projects
None yet
Development

No branches or pull requests

8 participants