[REVIEW]: Mayawaves: Python Library for Interacting with the Einstein Toolkit and the MAYA Catalog #6032
Comments
editorialbot
added
Jupyter Notebook
Perl
review
Shell
Track: 1 (AASS)
|
Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks. 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: |
|
@editorialbot check references |
|
|
|
Wordcount for |
|
|
@deborahferguson, already a job for you: could you fix the missing DOI (see above)? |
|
Okay I just fixed it! |
|
@eloisabentivegna can you add me as a reviewer here too? Thanks :) |
|
@editorialbot add @Sbozzolo as reviewer |
|
@Sbozzolo added to the reviewers list! |
Review checklist for @SbozzoloConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
|
Hi @deborahferguson, given the long time it took to find you an editor and reviewers, I already started looking at the repository and the paper. There are some important issues that would be good to address first (before the rest of the review can proceed). My first impression is that the documentation is rather minimal (as far I can see there's only a few jupyter notebooks and the function docstrings). Importantly:
In addition to this, it would be helpful to expand the documentation to discuss the various modules in more details, provide references/equations/conventions used, discuss features, et cetera. Other first impressions: The git repository is quite large (560 MB), entirely due to the files in the test folder (the code is 500 kB). It would be good to see if it is possible to reduce the size. The git history is almost empty, so I cannot judge about authorship of the software. Regarding the paper, it is well written. Here's some recommendations for content that should be included: I think it would be good to mention that Einstein Toolkit is open source (is maya open source)? Given the fragmented nature of Einstein Toolkit, it would be useful to explicitly mention which thorns are supported. The paper also does not survey the state of the art regarding packages for gravitational wave analysis in Einstein Toolkit. In particular, it would be important to to discuss what additional features mayawaves has compared to the two gravitational wave analysis Python packages that come with Einstein Toolkit: kuibit (of which I am author), and POWER. I am looking forward to reviewing your package! |
|
Thanks for the quick feedback! I'll work on adding in more documentation promptly. Thanks! |
|
@Sbozzolo In regards to the authorship, this was developed on an internal GitHub and when we decided to make it public, I moved it onto the normal GitHub. I chose not to port over all the git history due to security risks of early versions having local file paths, etc hardcoded into files (particularly in regards to tests and example simulations). That git history does still exist so maybe I can share screenshots or something of the contribution statistics |
I'll let @eloisabentivegna comment on how/if authorship should be verified. My comment was mostly to point out that I don't have much to say about it (and it is one of the boxes that need to be check). |
|
Good point, @Sbozzolo. Notice that non-code contributions can also be grounds for authorship in JOSS (see https://joss.readthedocs.io/en/latest/submitting.html#authorship). So, if the author list cannot clearly be established by inspecting the code repo, it is ultimately up to @deborahferguson to state who should be included (and perhaps provide a quick explanation why). Having said this, I would like to loop in @openjournals/aass-eics to confirm. |
Review checklist for @cjoanaConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
Thanks @eloisabentivegna! I agree with everything you've said here about our authorship policy. I think that given the unique context here (missing git history) it is useful to call this out, but we're generally happy to proceed given a summary like @deborahferguson's above. Thanks all! |
|
Thank you @deborahferguson! You addressed most of my comments! The main remaining ones:
|
|
Thanks for your suggestions @Sbozzolo I've updated the paper to include more citations and detail. I've also modified the code such that users can use it with the most recent python version as long as they don't want to do center-of-mass corrections. In the case they do want to do center-of-mass corrections, it will tell them they need to downgrade to 3.10. Thanks for that suggestion! I've also uploaded the recent updates to pypi and zenodo as a new version. |
Thanks! I fully ackowledge my bias here, but I think that the paper is not fairly and fully representing the state of the field. The paper reads
This implies that existing tools are limited, not versatile enough, and/or not user friendly enough. However, I think that all these problems are already solved by kuibit. Kuibit was also designed around the idea of exposing physical concepts instead of technical details, so I think it's unfair to say that mayawaves is unique in this (as the paragraph below the quoted one does). While I think I could be perfectly objective in my review, I do reckognize that I have personal interests here in this particular point. So, I'll let @eloisabentivegna comment on this. This is my last point before giving green light. Thanks again @deborahferguson for all your hard work during this review! |
|
@Sbozzolo Thanks for your candor, you're correct that it wasn't worded very well. Designing it for intuition and physical concepts are some of my favorite aspects of the library, but you're right that Kuibit is similar in that way and I shouldn't say otherwise. I've done some rephrasing that will hopefully be acceptable. Thanks for all your feedback, it's been very helpful. |
|
Thanks @Sbozzolo for the comments and for disclosing your bias -- I find your viewpoint useful nonetheless. My own perspective is no less biased (I have family connections to SimulationTools), but I have looked at the current wording and find it fair. It is, however, less descriptive than before. Can we find a description of Mayawaves' unique contributions that we can all agree upon? The creation of Mayawaves must have surely stemmed from shortcomings in the other solutions; the latter should not be understated. @deborahferguson: thanks for the effort so far; could you possibly add a sentence or two explaining your package's unique value to new users? |
|
I've done some additional rephrasing and added some text to make sure it is clear what the unique benefits of Mayawaves are, and I believe the second and third paragraphs of the Statement of Need describe it well. Part of the challenge has been that Mayawaves and Kuibit were being developed at the same time, clearly with a lot of the same ideas, and I want to be sure to give an accurate and fair representation of both. I appreciate all of your help to improve this paper and library! |
|
Thanks @deborahferguson. I understand the complexities of creating software in a very active field. The new text seems fair to me. @Sbozzolo, could you share your thoughts? @cjoana, it appears you are done with your checklist -- can you confirm your recommendation? |
|
Hi @eloisabentivegna, please read the follow keeping my bias in mind, and feel free to ignore it if you think my position it too biased. :) I think the new text is fair, but I don't think it answers the question "Do the authors describe how this software compares to other commonly-used packages?": the other packages are just mentioned and nothing is said about them. If I were to discuss the state of the field in software for post-processing Einstein Toolkit simulations, I would mention:
I think the more accurate representation of the state of the field is that there was an underlying issue with usability and user-friendlines that both |
|
Hi @eloisabentivegna, @deborahferguson Yes, I am done with the checklist, for me the paper is ready to go. |
|
@Sbozzolo, thanks for the detailed viewpoint. The description you give is extremely useful and will remain on the record for this review. At the same time, the Statement of Need and, to a lesser extent, the State of the Field often end up being somewhat subjective to who writes them and when. Others may not agree on the state-of-the-art assessment and the feeling that specific functionality is needed, and the review process is not meant to resolve these divergences but to confirm that the authors have fully documented their motivations for writing a given package at a given point in time. This now seems to have been done. If this is the last point on your list, can you provide your final recommendation for Mayawaves? |
|
Yes, @eloisabentivegna, I recommend mayawaves for publication in JOSS and I wholeheartely welcome this new user-friendly public code to the community's shared toolkit. |
|
@editorialbot check references |
|
|
@deborahferguson, could you inspect the missing DOIs listed above? I understand that a DOI is not applicable to every reference, but please include it wherever possible. |
|
Excellent! Thanks for all your work on this. I've added all the missing DOIs I could find. I still can't find ones for SimulationTools, PyCactus, or POWER. |
|
@editorialbot check references |
|
|
POWER: https://doi.org/10.1088/1361-6382/aa9cad (CQG publication) source code (currently) https://github.com/NCSAGravity/Gravitational_Waveform_Extractor SimulationTools: Ian may now what to use PyCactus: no idea, Wolfgang may have something other than the source code repo. Astrophysics code library has this https://ascl.net/2107.017 |
|
@editorialbot check references |
|
|
Thanks @rhaas80! I updated the citation for POWER |
|
Thanks @rhaas80 for filling the gaps, and @deborahferguson for updating the references. Unless @ianhinder and @wokast object, I would leave the remaining two references as they are, as I can't find a recommended DOId publication to cite for either package. |
|
@editorialbot generate pdf |
It is fine as it is; there is no other reference for SimulationTools. |
|
@deborahferguson, I've just noted a few text improvements in an issue under the code repo. Please take a look, and if happy apply and move on to the post-review checklist below. |
Post-Review Checklist for Editor and AuthorsAdditional Author Tasks After Review is Complete
Editor Tasks Prior to Acceptance
|
|
@deborahferguson, could you take a look at the tasks above, and proceed with the version/archive creation? |
Submitting author: @deborahferguson (Deborah Ferguson)
Repository: https://github.com/MayaWaves/mayawaves
Branch with paper.md (empty if default branch): paper
Version: 2023.8
Editor: @eloisabentivegna
Reviewers: @cjoana, @Sbozzolo
Archive: Pending
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
@cjoana, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review.
First of all you need to run this command in a separate comment to create the checklist:
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 ✨
Checklists
📝 Checklist for @Sbozzolo
📝 Checklist for @cjoana
The text was updated successfully, but these errors were encountered: