[REVIEW]: GRBoondi: A code for evolving Generalized Proca theories on arbitrary backgrounds #6888
Comments
editorialbot
added
C++
Mathematica
Python
review
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: |
|
|
Software report: Commit count by author: |
|
Paper file info: 📄 Wordcount for ✅ The paper includes a |
|
License info: ✅ License found: |
Review checklist for @SbozzoloConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
|
Hi @ShaunFell, I started my review and everything is looking great. Could you please clearify the relationship between GRBoondi and GRChombo? In terms of functionalities provided, my first impression is that GRBoondi would be optimally suited to be a module for GRChombo instead of being a separate codebase (GRBoondi even comes with a complete copy of GRChombo). Is there any intention to make GRBoondi an official module for GRChombo? Or, should GRBoondi be considered a hard fork of GRChombo? Having GRBoondi as a module would help with shared tooling (e.g. post-processing) and reduce the maintenance burden on your end. It would be really useful to have an end-to-end tutorial that explains how to run a simple example GRBoondi. Right now, most of the "getting started" is spread across multiple wiki pages and it is a little intimidating to figure out how to run a simple case. I'd suggest adding a page with a tutorial to run a simple example. In this tutorial, you can focus on just running something and making a simple visualization. Then, you can provide backlinks to the pages on "Parameter files", "Running Examples", "Visualization" for more in depth discussions. Could you also elaborate on the differences with |
|
Hi @ShaunFell I haven't had a chance to go through the code but I wanted to echo what @Sbozzolo said about the relationship between GRChombo and GRBoondi not being very clear. I would strongly suggest that you don't duplicate code from the GRChombo or GRDzhadzha repositories - it is fine for this code to have dependencies on them, in the same way as GRDzhadzha does on GRChombo, and this allows updates and optimisations that we make there to flow into your code. If there are aspects you found you needed to change in those repositories, I'd be happy to discuss making changes to them there to ensure compatibility, or finding a workaround. If you agree I would suggest that this is amended before we review the code in more detail. |
|
Hi @KAClough! |
|
Hi @Sbozzolo!
GRBoondi utilizes a lot of the source code from GRChombo, but also defines it's own functions for specifying the physical problems to that of generalized Proca around arbitrary spacetimes. While GRBoondi utilizes a lot of the GRChombo source, like the AMR interpolator and level classes, it also defines many of its own functionality, such as BaseProcaFieldLevel, since generalized Proca problems typically all share very similar structures when it comes to the code, like computing surface and volume integrals of the Proca norm, the Eulerian energy, etc. So broadly speaking, GRBoondi is very much it's own program that uses the AMR, level class, boundary code, etc. functionality of GRChombo/Chombo.
Sort of extending on what @KAClough mentioned, the GRChombo source will be removed from the GRBoondi source code. I did this since I had to modify some of the source code, but ended up creating my own classes and just left GRChombo in the GRBoondi source. I will remove this shortly with a pull request. So GRBoondi will required a fork of GRChombo. At the moment, I have no intention of making GRBoondi a module of GRChombo, though thats an interesting idea! generalized Proca theories are quite complex so keeping GRBoondi separate from GRChombo allows me to be quite versatile in changing the code for the needs of higher-order theories in the generalized Proca landscape.
ok, great idea! I will implement such a tutorial then.
Great idea, thanks!
GRDzhadzha is quite a generic toolkit for simulating any sort of matter field on generic fixed spacetimes. If one wants to simulate generalized Proca, they run into the same sort of problem as if they were to use GRChombo. Besides the lack of post-processing tools, users would have to manually rewrite a lot of the boilerplate code for each generalized Proca theory they simulate. This is basically the whole reason GRBoondi was created, to simplify a lot of this boilerplate code, making it much easier for users to study the generalized Proca landscape. For example, BaseProcaFieldLevel contains almost all the level class code that is common to all generalized Proca theories. This motif is shared throughout many of the classes in GRBoondi. |
|
@KAClough I've removed the GRChombo source code in ShaunFell/GRBoondi#15 and updated the main branch |
|
@editorialbot check repository |
|
Software report: Commit count by author: |
|
Paper file info: 📄 Wordcount for ✅ The paper includes a |
|
License info: ✅ License found: |
xuanxu
removed
the
waitlisted
|
Thanks @ShaunFell for the update. I'm very happy you can easily remove the copy of GRChombo! On GRDzhadzha, there is still a reasonable amount of duplication of core code from that in your repo... I don't feel as strongly that you should inherit from that code and I understand the desire to reduce dependencies, but if you were to link to it then any future tools we introduce there, like new fixed backgrounds or diagnostics (of which you already directly use some), could naturally flow into your code. I think as a minimum the README should acknowledge the use of parts of the GRDzhadzha code and the documentation should make clear the rationale for not inheriting from it. |
|
Hi @ShaunFell, I agree with @KAClough. I think that it would be a net positive for everyone to work more closely with the GRChombo collaboration and re-use and share as much code as possible. In any case, it should be very clear how GRChombo is used. As it stands, any update to GRChombo might end up breaking GRBoondi.
Choosing to move away from the released GRChombo has implications for the maintenance burden, so users should be provided with this information. |
|
Hi @KAClough! I decided against making GRDzhadzha a dependency since that would add to the large dependency list and make downloading GRBoondi a bit more involved. I only use a very small portion of the GRDzhadzha codebase and I modify some of the files myself, so including it instead as a dependency is a bit unnecessary. I added this mention to the GRBoondi wiki. @Sbozzolo, I updated GRBoondi to make GRChombo a dependency, so updates to GRChombo now naturally flow into GRBoondi. |
|
Software report: Commit count by author: |
|
Paper file info: 📄 Wordcount for ✅ The paper includes a |
|
License info: ✅ License found: |
|
Hi @warrickball , thank you for your comments. Is 1360 acceptable? I kind of running bare bones on some of the explanations.
Updated! Thanks for catching that |
|
Thanks! I'm happy with 1360. I'll aim to have a closer editorial look at the remaining text in the next few days. |
|
@editorialbot generate pdf |
|
Hi @ShaunFell — Thanks for getting the word count down. I've created an initial PR with a few small changes but the references generally need some work. In particular, many entries have incomplete author lists, where long lists are replaced with "& others" in the BibTeX. Please at least correct these to contain complete author lists, which should be available either from the relevant journal/archive website, or otherwise NASA ADS is a good source for bibliographic information. If you haven't already, for software you should see if you can find a recommended citation through the relevant documentation but note that this can give mixed quality. For VisIt, for example, there's a CITATION.cff file but the commented BibTeX is actually much richer than the uncommented contents of the file. |
|
Hi @warrickball, thank you for the pull request! The corrections look good, thanks for fixing them. I revamped the .bib file a bit and included proper citations with complete author lists. Let me know if you would like anything else changed. @editorialbot generate pdf |
|
@editorialbot generate pdf @ShaunFell the bot command has to be the first line of a comment for the bot to recognize it |
|
Hi @ShaunFell — I've just returned to this and opened another PR with what I hope are some final changes. Once those are merged I'll give the paper publication worfklow another test run. |
|
@editorialbot recommend-accept |
|
|
|
👋 @openjournals/aass-eics, this paper is ready to be accepted and published. Check final proof 👉📄 Download article If the paper PDF and the deposit XML files look good in openjournals/joss-papers#6101, then you can now move forward with accepting the submission by compiling again with the command |
|
@editorialbot accept |
|
|
Ensure proper citation by uploading a plain text CITATION.cff file to the default branch of your repository. If using GitHub, a Cite this repository menu will appear in the About section, containing both APA and BibTeX formats. When exported to Zotero using a browser plugin, Zotero will automatically create an entry using the information contained in the .cff file. You can copy the contents for your CITATION.cff file here: CITATION.cff
If the repository is not hosted on GitHub, a .cff file can still be uploaded to set your preferred citation. Users will be able to manually copy and paste the citation. |
|
🐘🐘🐘 👉 Toot for this paper 👈 🐘🐘🐘 |
|
🦋🦋🦋 👉 Bluesky post for this paper 👈 🦋🦋🦋 |
|
🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨 Here's what you must now do:
Any issues? Notify your editorial technical team... |
|
Thanks once again to @Sbozzolo & @KAClough for reviewing and @xuanxu for editing this submission! JOSS simply wouldn't be possible without its community of volunteers. Congratulations @ShaunFell, your paper has been published in JOSS! |
|
🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉 If you would like to include a link to your paper from your README use the following code snippets
This is how it will look in your documentation: We need your help! The 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:
|
|
Thank you once again to @KAClough and @Sbozzolo for their time and effort to help me improve GRBoondi! Thank you @xuanxu for facilitating the review and @warrickball for helping me polish the paper! |
Submitting author: @ShaunFell (Shaun D. B. Fell)
Repository: https://github.com/ShaunFell/GRBoondi
Branch with paper.md (empty if default branch): paper
Version: v1.0.0
Editor: @xuanxu
Reviewers: @Sbozzolo, @KAClough
Archive: 10.5281/zenodo.13982073
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
@Sbozzolo & @KAClough, 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 @xuanxu 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 @KAClough
The text was updated successfully, but these errors were encountered: