search

openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
707 stars 37 forks source link

[REVIEW]: GRDzhadzha: A code for evolving relativistic matter on analytic metric backgrounds #5956

Closed editorialbot closed 5 months ago

editorialbot commented 11 months ago

Submitting author: !--author-handle-->@dinatraykova<!--end-author-handle-- (Dina Traykova) Repository: https://github.com/GRChombo/GRDzhadzha.git Branch with paper.md (empty if default branch): Version: v1.0 Editor: !--editor-->@warrickball<!--end-editor-- Reviewers: @rashti-alireza, @ekwessel Archive: 10.5281/zenodo.10839756

Status

status

Status badge code:

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

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

@rashti-alireza & @ekwessel, 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:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @warrickball 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 @ekwessel

📝 Checklist for @rashti-alireza

editorialbot commented 11 months ago

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:

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf
editorialbot commented 11 months ago 
Software report:

github.com/AlDanial/cloc v 1.88  T=0.04 s (1607.0 files/s, 156715.2 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
C/C++ Header                    47            571            811           2609
TeX                              1             80              0           1000
C++                              6            108            166            713
YAML                             7             37              6            278
Python                           3             18              2            163
Markdown                         2             49              0            157
make                             5             47             27             82
-------------------------------------------------------------------------------
SUM:                            71            910           1012           5002
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 11 months ago

Wordcount for paper.md is 2157

editorialbot commented 11 months ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.21105/joss.03703 is OK
- 10.1103/PhysRevD.103.104006 is OK
- 10.1088/1361-6382/ac6fa9 is OK
- 10.3847/1538-4365/ac157b is OK
- 10.1103/PhysRevD.103.083501 is OK
- 10.1103/PhysRevD.103.044059 is OK
- 10.1007/JHEP03(2022)111 is OK
- 10.1103/PhysRevX.11.021053 is OK
- 10.1088/1475-7516/2021/05/027 is OK
- 10.1088/1361-6382/abb693 is OK
- 10.1088/1361-6382/ab939b is OK
- 10.1088/1361-6382/aba28b is OK
- 10.1088/1475-7516/2020/05/030 is OK
- 10.1088/1475-7516/2020/01/027 is OK
- 10.1103/PhysRevD.105.063517 is OK
- 10.1103/PhysRevD.100.086014 is OK
- 10.1088/1475-7516/2019/07/044 is OK
- 10.1088/1475-7516/2020/01/007 is OK
- 10.1103/PhysRevD.100.063014 is OK
- 10.1103/PhysRevD.99.064035 is OK
- 10.1142/S0218271819500147 is OK
- 10.1103/PhysRevD.99.104028 is OK
- 10.1103/PhysRevD.98.083020 is OK
- 10.1088/1361-6382/aaf43e is OK
- 10.1088/1475-7516/2018/10/005 is OK
- 10.1088/1361-6382/aad7f6 is OK
- 10.1103/PhysRevD.98.043004 is OK
- 10.1103/PhysRevD.99.044046 is OK
- 10.1088/1475-7516/2018/05/065 is OK
- 10.1103/PhysRevD.97.064036 is OK
- 10.1093/nsr/nwx116 is OK
- 10.1103/PhysRevLett.118.151103 is OK
- 10.1088/1475-7516/2017/03/055 is OK
- 10.1016/j.jcp.2016.12.059 is OK
- 10.1088/1475-7516/2017/09/025 is OK
- 10.1103/PhysRevLett.116.061102 is OK
- 10.1103/PhysRevLett.116.071102 is OK
- 10.1088/0264-9381/33/3/035010 is OK
- 10.1103/PhysRevD.93.124059 is OK
- 10.1103/PhysRevD.93.063006 is OK
- 10.1088/0264-9381/32/24/245011 is OK
- 10.1088/0264-9381/32/7/074001 is OK
- 10.1103/PhysRevD.85.124010 is OK
- 10.1088/0264-9381/29/11/115001 is OK
- 10.1088/0264-9381/29/12/124007 is OK
- 10.1103/PhysRevD.85.064040 is OK
- 10.1103/PhysRevD.82.024005 is OK
- 10.1103/PhysRevD.81.084052 is OK
- 10.1142/9789812834300_0200 is OK
- 10.1088/0264-9381/24/12/S05 is OK
- 10.1103/PhysRevD.77.024027 is OK
- 10.1103/PhysRevD.76.104015 is OK
- 10.1103/PhysRevLett.96.111101 is OK
- 10.1103/PhysRevLett.96.111102 is OK
- 10.1088/0264-9381/22/17/025 is OK
- 10.1016/j.cpc.2006.02.002 is OK
- 10.1103/PhysRevD.70.064011 is OK
- 10.1088/0264-9381/21/6/014 is OK
- 10.1016/S0010-4655(02)00847-0 is OK
- 10.5281/zenodo.4734670 is OK
- 10.1103/PhysRevD.59.024007 is OK
- 10.1103/PhysRevD.52.5428 is OK
- 10.1143/PTPS.90.1 is OK
- 10.1016/0021-9991(84)90073-1 is OK
- 10.1109/21.120081 is OK
- 10.1002/andp.19163540702 is OK
- 10.1103/PhysRevD.104.103014 is OK
- 10.1103/PhysRevLett.129.151102 is OK
- 10.1098/rsta.2017.0122 is OK
- 10.1088/1475-7516/2015/03/007 is OK
- 10.1103/PhysRevD.107.024035 is OK
- 10.1103/PhysRevD.105.104055 is OK
- 10.1088/1361-6382/ac10ee is OK
- 10.1088/1361-6382/accc6a is OK
- 10.1145/3311790.3396656 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 11 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

warrickball commented 11 months ago

Hi @rashti-alireza and @ekwessel, and thanks again for agreeing to review. This is the review thread for the paper. All of our correspondence will now happen here.

Please read the "Reviewer instructions & questions" in the first comment above, and generate your checklists by commenting @editorialbot generate my checklist on this issue. 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. We aim to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. We also encourage reviewers to submit issues and pull requests on the software repository. When doing so, please mention openjournals/joss-reviews#5956 so that the issue/PR is linked to this thread. Please also feel free to comment and ask questions on this thread. JOSS editors have found it 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 start whenever you can. JOSS reviews are iterative and the authors can start responding while you continue to review other parts of the submission.

Finally, don't hesitate to ask many any questions you might have about the process.

warrickball commented 11 months ago

Hi too to @dinatraykova, and apologies for the relatively long wait while we found an editor (me!) and reviewer for your submission, but we're at last ready to start!

ekwessel commented 11 months ago

Hello! Just checking in to apologize for the delay! I have looked over the submission, but had some other tasks pre-occupy me in the previous week. Those are now done, so I will begin making progress on the checklist items tomorrow

dinatraykova commented 11 months ago

Hi all! Thanks for agreeing to review our submission and no worries about the delay :)

ekwessel commented 10 months ago

Review checklist for @ekwessel

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

rashti-alireza commented 10 months ago

Hi all, hope you're all doing great. I have started the review.

rashti-alireza commented 10 months ago

Review checklist for @rashti-alireza

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

rashti-alireza commented 10 months ago

@dinatraykova : Regarding the "Contribution and authorship" checklist, as I examined the git log of the repository, I observed that some authors listed in the paper do not have any associated commits. Furthermore, based on the git log, it appears that certain authors mostly made changes to text and file names. Could you please provide clarification on these specific points? Thank you!

dinatraykova commented 10 months ago

@rashti-alireza Regarding the contributions, some of the authors contributed in writing the code at earlier stages, before it was moved to a separate repository and some authors have their own forks where they tested the code for specific problems and shared feedback with the developers, and where this was sufficiently significant we included them in the author list.

ekwessel commented 10 months ago

I have completed my review, and as far as I can see, everything looks good!

The biggest hold-up for me was figuring out how to install GRChombo (which I have never used before) on a new cluster. The process involved some trial and error, drawing on a good amount of experience configuring codes in HPC environments. However, for performant NR codes such as this, cluster-specific configuration is essential, and it is reasonable to expect any potential users ought to have the necessary technical skills. Additionally, I found the GRChombo documentation provided very helpful guidance. In particular the repository of common issues and their resolutions was crucial to enabling me to identify and fix issues with my configuration. So, while this slowed me down, I don't believe it is a issue. After GRChombo was installed and the tests were passing, installing GRDzhadzha based on the provided instructions in this repository was easy.

I built and ran the tests, which passed, as well as the examples, and was able to plot their output, which looks sensible. The only statement about performance in the paper involved a very rough comparison between comparable situations in GRChombo and GRDzhadzha: I was able to compare the runtime of one of the GRChombo examples to the similar BoostedBH example here, and can verify that the claims about the relative speedup are accurate.

I am therefore recommending acceptance of this code. Well done!

ekwessel commented 10 months ago

@dinatraykova I do have one small question: what inspired the name "GRDzhadzha"?

ekwessel commented 10 months ago

@editorialbot recommend-accept

editorialbot commented 10 months ago

I'm sorry @ekwessel, I'm afraid I can't do that. That's something only editors are allowed to do.

rashti-alireza commented 10 months ago

@dinatraykova I'm at the software paper checklist. The paper demonstrates a high standard of writing and aligns mostly with the criteria set by the JOSS journal. I do have a few points to be clarified to proceed. Please find the raised points in the following section:

rashti-alireza commented 10 months ago

@dinatraykova Since the code relies on analytically derived metrics , please provide me convergence testings of the Hamiltonian and momentum constraints for a black hole in the Kerr-Schild coordinates and a boosted black hole in isotropic Schwarzschild coordinates. Additionally, please ensure these tests conform to the finite difference scheme employed in the code. Ideally, I need a plot showing the constraints are decreased by increasing the resolution with a proper slope.

rashti-alireza commented 10 months ago

@dinatraykova Can you explain what the 'Automated tests' are? What do they actually test?

dinatraykova commented 9 months ago

@dinatraykova I do have one small question: what inspired the name "GRDzhadzha"?

@ekwessel hanks for your review! The name comes from the Bulgarian word for dadget, джа-джа (dzha-dzha), to keep in line with the naming of GRChombo/GRTeclyn, where combo and teclyn mean tool in Swahili and Welsh

dinatraykova commented 9 months ago

@dinatraykova I'm at the software paper checklist. The paper demonstrates a high standard of writing and aligns mostly with the criteria set by the JOSS journal. I do have a few points to be clarified to proceed. Please find the raised points in the following section:

  • [ ] Please provide citations for the sentence or statement takes place in the following lines:
  • line 17:"study questions in fundamental physics, such as the existence and properties..."
  • line 19:"...one imposes significant symmetries"
  • line 26:"...in many cases of interest..."
  • line 27:"in which case it is a reasonable approximation..."
  • line 47:"...can easily be adapted to other ..."
  • line 50:"an ADM decomposition..."
  • line 59:"extrapolating"
  • line 59:"Sommerfeld"

@rashti-alireza Thanks for the useful comments and questions! We have added citations or clarifications to the text as requested (with the exception of the statement in line 19:"...one imposes significant symmetries”, which is quite general, and so there is no specific citation that can be provided. )

  • [ ] Please be specific about what you mean by "significantly speeds up ..." at the line 34.

We specify this in more detail in the statement of need section.

  • [ ] The figure doesn't have any caption, please provide a descriptive caption and a citation.

Thanks for pointing this out, the caption was missed and is now added.

  • [ ] There is no actual citation link at the line 130.

This paper is still in preparation, so we removed the false link from the citation.

  • [ ] There are sections that are called "Key features of GRChombo" and "Key research projects using GRChombo" why "GRChombo"? and not "GRDzhadzha"?

This was a typo in a previous version of the draft, it's been corrected.

dinatraykova commented 9 months ago

@editorialbot generate pdf

editorialbot commented 9 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

dinatraykova commented 9 months ago

@dinatraykova Since the code relies on analytically derived metrics , please provide me convergence testings of the Hamiltonian and momentum constraints for a black hole in the Kerr-Schild coordinates and a boosted black hole in isotropic Schwarzschild coordinates. Additionally, please ensure these tests conform to the finite difference scheme employed in the code. Ideally, I need a plot showing the constraints are decreased by increasing the resolution with a proper slope.

@rashti-alireza This is what the tests in the Test folder do. For each example, they show that the time derivatives of the metric functions, and the Hamiltonian and momentum constraints for the initial data are really zero (see Tests/BoostedBHComplexScalarTest/BoostedBHScalarTest.cpp and Tests/KerrBHScalarTest/KerrBHScalarTest.cpp)

rashti-alireza commented 8 months ago

@editorialbot generate pdf

editorialbot commented 8 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

rashti-alireza commented 8 months ago

@dinatraykova Since the code relies on analytically derived metrics , please provide me convergence testings of the Hamiltonian and momentum constraints for a black hole in the Kerr-Schild coordinates and a boosted black hole in isotropic Schwarzschild coordinates. Additionally, please ensure these tests conform to the finite difference scheme employed in the code. Ideally, I need a plot showing the constraints are decreased by increasing the resolution with a proper slope.

@rashti-alireza This is what the tests in the Test folder do. For each example, they show that the time derivatives of the metric functions, and the Hamiltonian and momentum constraints for the initial data are really zero (see Tests/BoostedBHComplexScalarTest/BoostedBHScalarTest.cpp and Tests/KerrBHScalarTest/KerrBHScalarTest.cpp)

I would like to see a convergence test for the Hamiltonian constraint of a Kerr-Schild BH, i.e., the quantity $$H := R - K_{ij}K^{ij}+K^2. $$ I don't expect this quantity be "really zero" on a finite grid if $R$ and $K^{ij}$ are constructed from the Kerr-Schild metric. In this case $H$ should only converge to zero (unless you also compute $R$ and $K^{ij}$ analytically). Thanks!

warrickball commented 7 months ago

Hi @dinatraykova, thanks for your responses and work on the code following the reviewer comments. It sounds to me like this submission is quite close to acceptance if you can implement the convergence test @rashti-alireza has suggested. Let me know if there's anything that I might be able to help keep the review moving.

warrickball commented 7 months ago

@editorialbot generate pdf

editorialbot commented 7 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

warrickball commented 7 months ago

@editorialbot check references

editorialbot commented 7 months ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.21105/joss.03703 is OK
- 10.1103/PhysRevD.103.104006 is OK
- 10.1088/1361-6382/ac6fa9 is OK
- 10.3847/1538-4365/ac157b is OK
- 10.1103/PhysRevD.103.083501 is OK
- 10.1103/PhysRevD.103.044059 is OK
- 10.1007/JHEP03(2022)111 is OK
- 10.1103/PhysRevX.11.021053 is OK
- 10.1088/1475-7516/2021/05/027 is OK
- 10.1088/1361-6382/abb693 is OK
- 10.1088/1361-6382/ab939b is OK
- 10.1088/1361-6382/aba28b is OK
- 10.1088/1475-7516/2020/05/030 is OK
- 10.1088/1475-7516/2020/01/027 is OK
- 10.1103/PhysRevD.105.063517 is OK
- 10.1103/PhysRevD.100.086014 is OK
- 10.1088/1475-7516/2019/07/044 is OK
- 10.1088/1475-7516/2020/01/007 is OK
- 10.1103/PhysRevD.100.063014 is OK
- 10.1103/PhysRevD.99.064035 is OK
- 10.1142/S0218271819500147 is OK
- 10.1103/PhysRevD.99.104028 is OK
- 10.1103/PhysRevD.98.083020 is OK
- 10.1088/1361-6382/aaf43e is OK
- 10.1088/1475-7516/2018/10/005 is OK
- 10.1088/1361-6382/aad7f6 is OK
- 10.1103/PhysRevD.98.043004 is OK
- 10.1103/PhysRevD.99.044046 is OK
- 10.1088/1475-7516/2018/05/065 is OK
- 10.1103/PhysRevD.97.064036 is OK
- 10.1093/nsr/nwx116 is OK
- 10.1103/PhysRevLett.118.151103 is OK
- 10.1088/1475-7516/2017/03/055 is OK
- 10.1016/j.jcp.2016.12.059 is OK
- 10.1088/1475-7516/2017/09/025 is OK
- 10.1103/PhysRevLett.116.061102 is OK
- 10.1103/PhysRevLett.116.071102 is OK
- 10.1088/0264-9381/33/3/035010 is OK
- 10.1103/PhysRevD.93.124059 is OK
- 10.1103/PhysRevD.93.063006 is OK
- 10.1088/0264-9381/32/24/245011 is OK
- 10.1088/0264-9381/32/7/074001 is OK
- 10.1103/PhysRevD.85.124010 is OK
- 10.1088/0264-9381/29/11/115001 is OK
- 10.1088/0264-9381/29/12/124007 is OK
- 10.1103/PhysRevD.85.064040 is OK
- 10.1103/PhysRevD.82.024005 is OK
- 10.1103/PhysRevD.81.084052 is OK
- 10.1142/9789812834300_0200 is OK
- 10.1088/0264-9381/24/12/S05 is OK
- 10.1103/PhysRevD.77.024027 is OK
- 10.1103/PhysRevD.76.104015 is OK
- 10.1103/PhysRevLett.96.111101 is OK
- 10.1103/PhysRevLett.96.111102 is OK
- 10.1088/0264-9381/22/17/025 is OK
- 10.1016/j.cpc.2006.02.002 is OK
- 10.1103/PhysRevD.70.064011 is OK
- 10.1088/0264-9381/21/6/014 is OK
- 10.1016/S0010-4655(02)00847-0 is OK
- 10.5281/zenodo.4734670 is OK
- 10.1103/PhysRevD.59.024007 is OK
- 10.1103/PhysRevD.52.5428 is OK
- 10.1143/PTPS.90.1 is OK
- 10.1016/0021-9991(84)90073-1 is OK
- 10.1109/21.120081 is OK
- 10.1002/andp.19163540702 is OK
- 10.1103/PhysRevD.89.104059 is OK
- 10.1088/1742-6596/610/1/012044 is OK
- 10.1088/1361-6382/ab0587 is OK
- 10.1088/0004-637X/774/1/48 is OK
- 10.21468/SciPostPhysCore.3.2.007 is OK
- 10.1007/s10714-008-0661-1 is OK
- 10.1103/PhysRevD.67.084023 is OK
- 10.1103/PhysRevD.104.103014 is OK
- 10.1103/PhysRevLett.129.151102 is OK
- 10.1098/rsta.2017.0122 is OK
- 10.1088/1475-7516/2015/03/007 is OK
- 10.1103/PhysRevD.107.024035 is OK
- 10.1103/PhysRevD.105.104055 is OK
- 10.1088/1361-6382/ac10ee is OK
- 10.1088/1361-6382/accc6a is OK
- 10.1145/3311790.3396656 is OK

MISSING DOIs

- None

INVALID DOIs

- None
dinatraykova commented 7 months ago

@dinatraykova Since the code relies on analytically derived metrics , please provide me convergence testings of the Hamiltonian and momentum constraints for a black hole in the Kerr-Schild coordinates and a boosted black hole in isotropic Schwarzschild coordinates. Additionally, please ensure these tests conform to the finite difference scheme employed in the code. Ideally, I need a plot showing the constraints are decreased by increasing the resolution with a proper slope.

@rashti-alireza This is what the tests in the Test folder do. For each example, they show that the time derivatives of the metric functions, and the Hamiltonian and momentum constraints for the initial data are really zero (see Tests/BoostedBHComplexScalarTest/BoostedBHScalarTest.cpp and Tests/KerrBHScalarTest/KerrBHScalarTest.cpp)

I would like to see a convergence test for the Hamiltonian constraint of a Kerr-Schild BH, i.e., the quantity H:=R−KijKij+K2. I don't expect this quantity be "really zero" on a finite grid if R and Kij are constructed from the Kerr-Schild metric. In this case H should only converge to zero (unless you also compute R and Kij analytically). Thanks!

@rashti-alireza Maybe my previous response wasn't very clear, the convergence tests that we do show that the norm of the constraints converges to zero (see line 230 here: https://github.com/GRTLCollaboration/GRDzhadzha/blob/main/Tests/KerrBHScalarTest/KerrBHScalarTest.cpp )

Hope this clears things up :)

warrickball commented 7 months ago

Hi @rashti-alireza, does the convergence test that @dinatraykova linked satisfy you? I know little about numerical GR but I can at least see that the code snippet is a convergence test. I might be misunderstanding the jargon but is it an issue for you that this is a convergence test on the norm of the constraints, rather than a convergence test of the single constraint that you highlighted above?

As an aside for @dinatraykova, I just a look myself that the tests are indeed automated but noticed that the ifort build started failing about four months ago.

rashti-alireza commented 7 months ago

@dinatraykova Since the code relies on analytically derived metrics , please provide me convergence testings of the Hamiltonian and momentum constraints for a black hole in the Kerr-Schild coordinates and a boosted black hole in isotropic Schwarzschild coordinates. Additionally, please ensure these tests conform to the finite difference scheme employed in the code. Ideally, I need a plot showing the constraints are decreased by increasing the resolution with a proper slope.

@rashti-alireza This is what the tests in the Test folder do. For each example, they show that the time derivatives of the metric functions, and the Hamiltonian and momentum constraints for the initial data are really zero (see Tests/BoostedBHComplexScalarTest/BoostedBHScalarTest.cpp and Tests/KerrBHScalarTest/KerrBHScalarTest.cpp)

I would like to see a convergence test for the Hamiltonian constraint of a Kerr-Schild BH, i.e., the quantity H:=R−KijKij+K2. I don't expect this quantity be "really zero" on a finite grid if R and Kij are constructed from the Kerr-Schild metric. In this case H should only converge to zero (unless you also compute R and Kij analytically). Thanks!

@rashti-alireza Maybe my previous response wasn't very clear, the convergence tests that we do show that the norm of the constraints converges to zero (see line 230 here: https://github.com/GRTLCollaboration/GRDzhadzha/blob/main/Tests/KerrBHScalarTest/KerrBHScalarTest.cpp )

Hope this clears things up :)

@dinatraykova, Great! that answers my question now.

rashti-alireza commented 7 months ago

Hi @rashti-alireza, does the convergence test that @dinatraykova linked satisfy you? I know little about numerical GR but I can at least see that the code snippet is a convergence test. I might be misunderstanding the jargon but is it an issue for you that this is a convergence test on the norm of the constraints, rather than a convergence test of the single constraint that you highlighted above?

As an aside for @dinatraykova, I just a look myself that the tests are indeed automated but noticed that the ifort build started failing about four months ago.

Hi @warrickball, everything is clear now and all of my questions have been answered adequately. The authors did a great work developing the GRDzhadzha code. The code has great potentials and I highly recommend it for the publication. Great job!

warrickball commented 7 months ago

Thanks @rashti-alireza and @ekwessel, and congratulations @dinatraykova on an accepted paper! I'll generate a post-review checklist of items for me to check off before the paper can be published.

warrickball commented 7 months ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

warrickball commented 7 months ago

@editorialbot generate pdf

editorialbot commented 7 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

warrickball commented 7 months ago

Hi again @dinatraykova, I've proposed a few small changes to the paper but a few more changes are required that I'll tick off as we resolve.

should be

(see e.g Barack et al., 2019; Barausse et al., 2015; Bertone et al., 2020; Macedo et al., 2013)

I remember this being tricky so let me know if you have trouble. See the JOSS docs and pandoc docs for the citation options.

dinatraykova commented 6 months ago

Thanks again @rashti-alireza and @ekwessel for your comments and suggestions! And thanks @warrickball for you editorial help! I've addressed your comments above in this pull request https://github.com/GRTLCollaboration/GRDzhadzha/pull/21 For the Chombo reference, I added a url and the full author list, which I think makes it easier to find.

Regarding the failing intel test, that started happening recently due to some changes in the main GRChombo code, which someone is working on resolving.

warrickball commented 6 months ago

I've addressed your comments above in this pull request https://github.com/GRTLCollaboration/GRDzhadzha/pull/21

I've had a look at the paper produced in the JOSS workflow run and the citations are still a mixed bag of parentheses. e.g.

I know these are tricky in the JOSS source format, so I'll have a go now at resolving these and, presuming I succeed (which is not guaranteed!), I'll open a PR against your branch with the other changes.

warrickball commented 6 months ago

I've opened https://github.com/GRTLCollaboration/GRDzhadzha/pull/22 to fix ~almost~ all the citations. ~The citations in footnote 1 aren't quite right but I'll consult the other JOSS editors about this.~ Citations in footnote 1 should now be fixed. (Thanks Marcel!)

I'll wait until I re-generate the PDF in this issue (after those PRs https://github.com/GRTLCollaboration/GRDzhadzha/pull/21 and https://github.com/GRTLCollaboration/GRDzhadzha/pull/22 are merged) before I tick the other editorial items in my list above but I can see in my local build that you've addressed them. Thanks!

dinatraykova commented 6 months ago

@warrickball Thanks so much with the help with the citation styling! I've merged PRs 21 and 22

dinatraykova commented 6 months ago

@editorialbot generate pdf

editorialbot commented 6 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

warrickball commented 6 months ago

@editorialbot check references