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]: kuibit: Analyzing Einstein Toolkit simulations with Python #3099

Closed
40 tasks done
whedon opened this issue Mar 12, 2021 · 86 comments
Closed
40 tasks done

[REVIEW]: kuibit: Analyzing Einstein Toolkit simulations with Python #3099

whedon opened this issue Mar 12, 2021 · 86 comments
Assignees
Labels
accepted published Papers published in JOSS Python recommend-accept Papers recommended for acceptance in JOSS. review

Comments

@whedon
Copy link

whedon commented Mar 12, 2021

Submitting author: @Sbozzolo (Gabriele Bozzola)
Repository: https://github.com/Sbozzolo/kuibit/
Version: 1.0.0
Editor: @eloisabentivegna
Reviewer: @Yurlungur, @eloisabentivegna
Archive: 10.5281/zenodo.4681119

⚠️ 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/fab8c50b7cac194fcf5d5ce8b0e22d9d"><img src="https://joss.theoj.org/papers/fab8c50b7cac194fcf5d5ce8b0e22d9d/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/fab8c50b7cac194fcf5d5ce8b0e22d9d/status.svg)](https://joss.theoj.org/papers/fab8c50b7cac194fcf5d5ce8b0e22d9d)

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

@Yurlungur & @eloisabentivegna, 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 @eloisabentivegna 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 @Yurlungur

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 (@Sbozzolo) 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 @eloisabentivegna

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 (@Sbozzolo) 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 Mar 12, 2021

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

@whedon
Copy link
Author

whedon commented Mar 12, 2021

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

Can't find any papers to compile :-(

@whedon
Copy link
Author

whedon commented Mar 12, 2021

Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.68 s (130.3 files/s, 36408.4 lines/s)
--------------------------------------------------------------------------------
Language                      files          blank        comment           code
--------------------------------------------------------------------------------
Python                           37           4153           7062           8399
reStructuredText                 28            587            442           1449
Jupyter Notebook                  6              0           1518            274
Markdown                          4             62              0            210
Bourne Again Shell                6             30             54             93
INI                               3              3              0             79
YAML                              1             13              3             65
TOML                              2             13              3             51
make                              1              4              7              9
--------------------------------------------------------------------------------
SUM:                             88           4865           9089          10629
--------------------------------------------------------------------------------


Statistical information for the repository '1adefaf928447035a5a9a48d' was
gathered on 2021/03/12.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Gabriele Bozzola               203         27319           7703           99.99
deepsource-autofix[b             1             0              2            0.01

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
Gabriele Bozzola          19614           71.8          4.6               19.14

@Yurlungur Yurlungur self-assigned this Mar 12, 2021
@Yurlungur
Copy link

@Sbozzolo Overall the paper reads quite well! One thing I noticed is that there is not much comparison to other visualization and analysis tools. For example, how does kuibit compare to visit, yt, or even pycactus? It may also be enough to mention that many users of the Einstein Toolkit write their own visualization pipelines by hand. I have created an issue to discuss on your repo here.

@Sbozzolo
Copy link

Sbozzolo commented Mar 14, 2021

Thanks, I added a paragraph that lists all the software officially recommended in this page https://docs.einsteintoolkit.org/et-docs/Analysis_and_post-processing.

The only other software that is comparable to kuibit is SimulationTools, but it requires Mathematica (non-free). Most of the other software for analysis appears to be outdated, unmaintained, and/or lacking documentation (including PyCactus). Tools like visit cannot be used to do quantitative analysis but only visualization. So, I think that kuibit is unique, especially in its mission of being a package accessible to newcomers (and thus, its emphasis on documentation, tutorials, examples, comments in the code, and in requiring no proprietary software).

@Yurlungur
Copy link

Thanks, @Sbozzolo Yes, that looks great.

@Yurlungur
Copy link

Your contribution rules also look quite good. I added an issue with one question I had about them here.

@Sbozzolo
Copy link

Thanks, I updated the CONTRIBUTING document with a detailed list of the steps to contribute.

@Yurlungur
Copy link

Perfect!

@Sbozzolo
Copy link

@Yurlungur, @eloisabentivegna

I don't want to put any pressure on you, and I fully understand that JOSS is operating in "reduced service mode", but I wanted to let you know that by April 14th we will have to submit a end-of-year report for funding/computational time. We would love to include this project as part of that report, so it would be great if the paper was published before then. In case it won't, I will submit it to arXiv and use the arXiv number, so it is not a big deal (but I am sure you will understand that a published paper is better).

Please, don't take this message as a "request to do your review quickly". I know you are volunteering your time, and I fully respect that. And of course, I don't want the review to be rushed, as the process will improve kuibit. I just wanted you to know that if this paper will be published in the next three weeks, that would help our group.

Thanks!

@Yurlungur
Copy link

@Sbozzolo thanks for letting me know. I anticipate getting done well before that time, though I don't know how quickly JOSS publications appear after review.

@eloisabentivegna
Copy link

@Sbozzolo, thanks for bringing this up. I also think the review stage should complete well before your deadline, and the remaining stages are usually quite lightweight.

@eloisabentivegna
Copy link

@whedon generate pdf from branch joss-paper

@whedon
Copy link
Author

whedon commented Mar 25, 2021

Attempting PDF compilation from custom branch joss-paper. Reticulating splines etc...

@whedon
Copy link
Author

whedon commented Mar 25, 2021

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

@whedon
Copy link
Author

whedon commented Mar 26, 2021

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

@whedon
Copy link
Author

whedon commented Mar 26, 2021

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

@eloisabentivegna
Copy link

@Sbozzolo, I am almost done with the code review.

I have a few suggestions regarding the paper text:

  • The Einstein Toolkit should always have a determinate article, e.g. "made by Einstein Toolkit" -> "made by the Einstein Toolkit" on line 28.
  • On line 55, "visit" -> "VisIt"
  • The citation label for Frontera should be corrected
  • The description of a typical workflow seems to imply that Simulation Factory is required to use kuibit. Is this the case? If so, it should be stated clearly.

I am also not sure I understand the note

(This notebook is meant to be converted in Sphinx documentation and not used directly.)

in the tutorial notebooks. Are these really not supposed to be used as tutorials? If not, what other document can users refer to for real-world examples?

@eloisabentivegna
Copy link

@Yurlungur, could you post an update?

@Sbozzolo
Copy link

Sbozzolo commented Apr 6, 2021

@whedon generate pdf from branch joss-paper

@whedon
Copy link
Author

whedon commented Apr 6, 2021

Attempting PDF compilation from custom branch joss-paper. Reticulating splines etc...

@whedon
Copy link
Author

whedon commented Apr 6, 2021

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

@Sbozzolo
Copy link

Sbozzolo commented Apr 6, 2021

Thanks, I took care of your comments. I added a sentence to say that kuibit does not assume any specific simulation workflow but gathers information from filenames and metadata.

I am also not sure I understand the note

(This notebook is meant to be converted in Sphinx documentation and not used directly.)

in the tutorial notebooks. Are these really not supposed to be used as tutorials?

The documentation notebooks are generated as part of the continuous integration pipeline using data that I include in the repo. They serve the purpose of showing the basic features available and how they should be used, but, due to technical limitations, the data included is very minimal. There is little point in running the notebooks interactively, as some of the features that are not used in the notebook will fail, given that the data is not realistic. If a user were to try to explore these other features, they would probably end up confused. In practice, I used jupyter only as a way to produce documentation that mixes code, comments, and output.

One can find real-world code looking at the experimental branch, which contains a large number of examples. The reason these are in the experimental branch is because all these scripts rely on at least one additional modules, argparse_helper, which I want to extend further before merging it back to master. This module is used to work with command-line arguments, and does not affect much the rest of the code, so the files are very good examples to learn how to work with kuibit. They are as real-world as it gets: if one is using the experimental branch, they can be used directly to do science. One can find this piece of information in the documentation. The readme of the repo also tries to make users aware of the experimental branch.

Since you couldn't find the examples, I guess I should make it more explicit.

@Yurlungur
Copy link

@Yurlungur, could you post an update?

My apologies---I fell ill last week and am only now catching up. I have much of the code review still to do. I can commit to finishing it by the end of the week.

@eloisabentivegna
Copy link

My apologies---I fell ill last week and am only now catching up. I have much of the code review still to do. I can commit to finishing it by the end of the week.

Thanks, @Yurlungur!

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

whedon commented Apr 12, 2021

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

 Can't find any papers to compile :-(

@openjournals openjournals deleted a comment from whedon Apr 12, 2021
@openjournals openjournals deleted a comment from whedon Apr 12, 2021
@eloisabentivegna
Copy link

@whedon accept from branch joss-paper

@whedon
Copy link
Author

whedon commented Apr 12, 2021

Attempting dry run of processing paper acceptance...

@whedon
Copy link
Author

whedon commented Apr 12, 2021

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

Check final proof 👉 openjournals/joss-papers#2222

If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#2222, 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 

@whedon
Copy link
Author

whedon commented Apr 12, 2021

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

OK DOIs

- 10.5281/zenodo.3866075 is OK
- 10.1007/3-540-36569-9_13 is OK
- doi:10.1088/0264-9381/29/11/115001 is OK
- 10.1103/PhysRevD.72.024028 is OK
- 10.1088/1361-6382/aa9cad is OK
- 10.1145/3311790.3396656 is OK

MISSING DOIs

- None

INVALID DOIs

- None

@eloisabentivegna
Copy link

Thanks @Sbozzolo for the nice package, and thanks @Yurlungur for an exceptionally thorough review! I appreciate your collaboration.

@Yurlungur
Copy link

My pleasure. I'm very excited to see Kuibit enter the Einstein Toolkit ecosystem!

@kyleniemeyer
Copy link

Hi @Sbozzolo, I am the AEIC on duty this week doing some final checks before accepting.

I noticed that the Guercilena citation might have the incorrect title or URL, can you double check that one? I think it should be rugutils.

@Sbozzolo
Copy link

Thank you very much, @eloisabentivegna and @Yurlungur !

@kyleniemeyer, yes, thanks, I just pushed a new version. I also took the chance to add the Cactus citation inline when I mention Cactus.

@kyleniemeyer
Copy link

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Apr 13, 2021

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

 Can't find any papers to compile :-(

@kyleniemeyer
Copy link

@whedon generate pdf from branch joss-paper

@whedon
Copy link
Author

whedon commented Apr 13, 2021

Attempting PDF compilation from custom branch joss-paper. Reticulating splines etc...

@whedon
Copy link
Author

whedon commented Apr 13, 2021

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

@kyleniemeyer
Copy link

@whedon accept from branch joss-paper

@whedon
Copy link
Author

whedon commented Apr 13, 2021

Attempting dry run of processing paper acceptance...

@whedon
Copy link
Author

whedon commented Apr 13, 2021

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

OK DOIs

- 10.5281/zenodo.3866075 is OK
- 10.1007/3-540-36569-9_13 is OK
- doi:10.1088/0264-9381/29/11/115001 is OK
- 10.1103/PhysRevD.72.024028 is OK
- 10.1088/1361-6382/aa9cad is OK
- 10.1145/3311790.3396656 is OK

MISSING DOIs

- None

INVALID DOIs

- None

@whedon
Copy link
Author

whedon commented Apr 13, 2021

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

Check final proof 👉 openjournals/joss-papers#2224

If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#2224, 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 

@kyleniemeyer
Copy link

@whedon accept deposit=true from branch joss-paper

@whedon whedon added accepted published Papers published in JOSS labels Apr 13, 2021
@whedon
Copy link
Author

whedon commented Apr 13, 2021

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

@whedon
Copy link
Author

whedon commented Apr 13, 2021

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

@whedon
Copy link
Author

whedon commented Apr 13, 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.03099 joss-papers#2225
  2. Wait a couple of minutes, then verify that the paper DOI resolves https://doi.org/10.21105/joss.03099
  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...

@kyleniemeyer
Copy link

Congratulations @Sbozzolo on your article's publication in JOSS!

Many thanks to @Yurlungur for reviewing this, and @eloisabentivegna for editing (and reviewing).

@whedon
Copy link
Author

whedon commented Apr 13, 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.03099/status.svg)](https://doi.org/10.21105/joss.03099)

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

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

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
Labels
accepted published Papers published in JOSS Python recommend-accept Papers recommended for acceptance in JOSS. review
Projects
None yet
Development

No branches or pull requests

5 participants