[REVIEW]: RelativisticDynamics.jl: Relativistic Spin-Orbital 1 Dynamics in Julia

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@tomkimpson@dfm<!--end-editor-- Reviewers: @langfzac, @tamasgal Archive: 10.5281/zenodo.8412240

Status

Status badge code:

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

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

@farr & @duetosymmetry, 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 @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 ✨

Checklists

📝 Checklist for @langfzac

📝 Checklist for @tamasgal

editorialbot commented 1 year 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 1 year ago

Software report:

github.com/AlDanial/cloc v 1.88  T=0.03 s (1142.6 files/s, 178939.7 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Julia                           18            619            121           1173
Markdown                         8            126              0            187
YAML                             6              5              7            141
TOML                             3             28              1            131
TeX                              1             12              0            130
Jupyter Notebook                 2              0           3206             64
-------------------------------------------------------------------------------
SUM:                            38            790           3335           1826
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository

editorialbot commented 1 year ago

Wordcount for paper.md is 1100

editorialbot commented 1 year ago

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

OK DOIs

- 10.1007/s10714-010-0939-y is OK
- 10.1098/rspa.1951.0200 is OK
- 10.1007/BF02734579 is OK
- 10.1093/mnras/stz389 is OK
- 10.1093/mnras/staa2103 is OK
- 10.1093/mnras/stz845 is OK
- 10.1051/0004-6361/202038561 is OK
- 10.1175/MWR-D-16-0228.1 is OK

MISSING DOIs

- None

INVALID DOIs

- None

editorialbot commented 1 year ago

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

dfm commented 1 year ago

@farr, @duetosymmetry — This is the review thread for the paper. All of our correspondence will happen here from now on. Thanks again for agreeing to participate!

👉 Please read the "Reviewer instructions & questions" in the first comment above, and generate your checklists by commenting @editorialbot generate my checklist on this issue ASAP. 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#4992 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 try to make a start 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. Please get your review started as soon as possible!

duetosymmetry commented 1 year ago

Review checklist for @duetosymmetry

Conflict of interest

[x] 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

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the https://github.com/tomkimpson/RelativisticDynamics.jl?
[x] 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 (@tomkimpson) 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
[x] Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
[ ] Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
[x] Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

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, who the target audience is, and its relation to other work?
[ ] 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?

farr commented 1 year ago

Review checklist for @farr

Conflict of interest

[x] 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

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[ ] Repository: Is the source code for this software available at the https://github.com/tomkimpson/RelativisticDynamics.jl?
[ ] 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 (@tomkimpson) 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
[ ] Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
[ ] Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
[ ] Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

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, who the target audience is, and its relation to other work?
[ ] 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?

dfm commented 1 year ago

@farr, @duetosymmetry, @tomkimpson — Happy new year! I'm writing to check in on the progress of this review, and to keep it on your radars. Please let me know if there are any major stoppers or if there's anything I can do to help move things along. Thanks!

dfm commented 1 year ago

@farr, @duetosymmetry, @tomkimpson — I'm checking in again to see what the status is on this review. Please let me know if you are running into any issues or if there's anything I can do to keep things rolling!

duetosymmetry commented 1 year ago

Hi all, sorry for being extremely slow... I've had a lot of responsibilities lately and let this one slide. Please forgive me! To avoid further delays let me just drop some initial thoughts here. Again sorry that this is not as thorough as it should be, but I thought that getting started with something is better than delaying.

First, I think the name RelativisticDynamics.jl is much too general for what is actually happening in this package. Lots of things could be relativistic dynamics. It could be fully nonlinear numerical relativity, or it could be simple post-Newtonian. This package is specifically for solving the MPD equations on the Kerr background, not more or less.

If the goal is for the package to become more flexible in the future — for example, solving the MPD equations on other exact spacetimes, or perturbatively away from Kerr — then it must be organized in such a way that a user could swap out the background metric with ease. That is impossible right now. All of the information for a background metric — code for computing contra- and covariant metric components, Christoffels, components of Riemann — should be collected together in a single piece of data.

The structure of metric.jl is rather messy (also, this is specific to Kerr, not any metric; and it's not just the metric, it's also connection coefficients and curvature). We have some functions whose names encode metric components, e.g. metric_g14. Then covariant_metric calls all of these. On the other hand, christoffel just computes all the Christoffel coefficients, without dispatching to anything like christoffel_111. Same thing for riemann. That is of course better. I don't know why there should be separate metric_gAB functions.

In some places in this code, there is reference to RelativisticDynamics.delta and RelativisticDynamics.sigma, while in other places it's just delta and sigma. I'm not sure why these are relegated to useful_functions.jl, or what determines why the functions in there ended up there. I would also recommend to use the relativists' naming to distinguish between the Levi-Civita symbol (which takes values either ±1 or 0) versus the Levi-Civita tensor (which transforms correctly under coordinate transformations).

Storage in Rtensor is extremely wasteful. There are only 10 independent quantities, but Rtensor allocates 256. This can be avoided be everywhere rewriting Riemann in terms of its electric/magnetic decomposition, both of these STF tensors having only 5 independent components (though needing to store 6, or more likely 9; still a huge improvement). This is not really a big point, since we're integrating an ODE, not solving a PDE. But still, I thought I should bring up these kind of design decisions.

I don't know why there is a separate schwarzchild_covariant_riemann function — and why does it have the spin, a, as an argument, when Schwarzschild is non-spinning? I would have thought that to make it obviously correct, one should just dispatch to riemann but with a=0, and lower the first index with the metric.

The documentation in orbit.jl refers to a file parameters.jl but there is no such file.

In universal_constants.jl, the speed of light should not be 3e8. It is defined to be exactly 299792458 m/s in SI units. In that same file: a capital G should probably be used for Newton's G. However, G is only measured to a level of 2*10^{-5}, and the mass of the Sun is similarly uncertain. However the product GM☉ is known to a level of about 10^{-10}. It should be clear how to rewrite things so that you only ever use the product GM☉ instead of either G or M_☉ by themselves (as is done in the pulsar timing and GR communities).

There are really many more issues about the clarity and organization of the code along these same lines, so I will not try to enumerate any more. In general, I think the code should organized better.

Turning to the documentation and article: I think the issue of spin supplementary conditions needs to be discussed much, much more. The SSCs are the main point of confusion when it comes to the MPD equations. Some statements you make in the code / documentation are only true for one particular SSC. It's fine to stick to one, as long as you make it clear that no others are allowed, and point out which statements are specific to the SSC you chose. It would be helpful to point to a specialist article about different SSCs.

Going back to the big picture: If the point of a specialized package like this one is for spinning bodies just in the Kerr spacetime, and you want to have high precision over very long times, then it would be better to use an action-angle formulation (see e.g. Vojtech Witzany's papers). That is only appropriate for the first order in spin, but as the article rightly mentions, the multipole expansion of the small body has already been truncated to dipole order — so the MPD equations are already an expansion. Of course the AA formulation is specific to Kerr. It would still be relevant spacetimes close to Kerr, perturbation theory. On the other hand to make it totally general, for any background spacetime, you can't use AA variables. I think it would be appropriate, in the Statement of Need, to discuss why this has been formulated as a generic ODE integration problem, rather than specializing to AA variables.

Sorry again that I waited so long to say anything. I hope this gives some useful directions for improvement of both the code and documentation. We can continue to discuss.

tomkimpson commented 1 year ago

Thanks @duetosymmetry for the thorough comments. No worries re the delay - I understand you're busy and appreciate you taking the time to review. I'll open a PR and start working through your comments and ping for any further discussion. Cheers!

dfm commented 1 year ago

Thanks to @duetosymmetry for these comments and suggestion, and to @tomkimpson for starting to work through them!

I want to also ping @farr to keep this on the radar. Please try to start going through the checklist ASAP to see if you have some comments to add to what @duetosymmetry has so far. Many thanks!!

tomkimpson commented 1 year ago

Thanks all for the help with this. Just giving this issue a bump - the comments have all been addressed on my side

duetosymmetry commented 1 year ago

Ping @farr for any review.

Question for @dfm: Should we be reviewing the PR tomkimpson/RelativisticDynamics.jl#41, or should @tomkimpson merge it in if he sees fit, and we do another round of refereeing back here? (Tangentially related, are the Julia docs also being generated from the PR branch? I can't find those separately).

dfm commented 1 year ago

Thanks for checking in @tomkimpson and @duetosymmetry! I'll email @farr to remind him as well.

Should we be reviewing the PR https://github.com/tomkimpson/RelativisticDynamics.jl/pull/41, or should @tomkimpson merge it in if he sees fit, and we do another round of refereeing back here? (Tangentially related, are the Julia docs also being generated from the PR branch? I can't find those separately).

Good question! These reviews can progress either way. It's actually recommended to do as much of the iteration as you can on PRs and issues in the parent repository, rather than directly in this thread. So it certainly could be a good approach to go through that PR directly, but we're flexible, so please use whichever approach works best for you!

dfm commented 1 year ago

Just an update that I'm not getting anything from @farr even over email so I'm working on finding another reviewer to replace him.

In the meantime, I wanted to check in with @tomkimpson and @duetosymmetry to see where things stand with the currently open discussion points. Please let me know if there are any issues or major stoppers, and if you have a sense of the timeline for both of you working through the rest of the review. Thanks!

tomkimpson commented 1 year ago

Thanks @dfm. All of @duetosymmetry comments were address in https://github.com/tomkimpson/RelativisticDynamics.jl/pull/41 . I have been leaving this PR open for more comments, but if @duetosymmetry is happy I will close it while we wait for the new reviewer.

One question for @dfm : it was recommended to change the package name to be less general. This is obviously straightforward but are there any issue on the JOSS side if I update the github repo name?

dfm commented 1 year ago

One question for @dfm : it was recommended to change the package name to be less general. This is obviously straightforward but are there any issue on the JOSS side if I update the github repo name?

@tomkimpson — I'm so sorry for the delayed response! No - there is no problem with this. Just ping me once you've changed the name!

dfm commented 1 year ago

@editorialbot add @langfzac as reviewer

🎉 Thanks @langfzac for agreeing to step in as another reviewer!! Please take a look above at the conversation so far and let me know if you have any questions! In particular, take a look at my comment above for more info abut how this review should go. A good place to start is by commenting @editorialbot generate my checklist on this thread and then checking the first couple of boxes. Thanks again!!

editorialbot commented 1 year ago

@langfzac added to the reviewers list!

langfzac commented 1 year ago

Review checklist for @langfzac

Conflict of interest

[x] 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

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the https://github.com/tomkimpson/RelativisticDynamics.jl?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@tomkimpson) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
[x] Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
[x] Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
[x] Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] 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

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
[x] 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

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] 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, who the target audience is, and its relation to other work?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] 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?

langfzac commented 1 year ago

Hi all, I have some initial comments after reading through the paper/docs, and trying out the example code.

Paper:

The paper mentions that this code is meant to fill the space between full NR and simple post-Newtonian treatments. It would be good to include references to existing codes.

Documentation:

"How to run RelativisticDynamics.jl" section:
- The user is pointed to a file src/default_parameters.jl, which (I assume) should be src/system_parameters.jl.
- What is the run_speedy() function? I don't see it defined anywhere in the repo.
"Visualisation of the solution":
- The 3D PlotTrajectory() throws an error: UndefVarError: 'plt' not defined.
- StackedPlot throws an error: type Model has no field constant

Example Notebook:

Supplying the NF=Float32 keyword to orbit() throws an error: No matching function wrapper was found! on Julia v1.9. However, it does work on v1.7.
I think this is already known, but autodiff appears to be broken. It looks like in the dts_comments_1 branch, the notebook has an updated autodiff section, which is marked as "TODO".

Tests/Continuous Integration:

I think the tests should be running on at least the Long-Term-Support version of Julia. Preferably, both LTS and the latest release. Currently, they only run on v1.7.
Since the ability to use autodiff is a highlighted feature, there should be a relevant unit test for orbit().
Consider using a doctest for the "how to run" example, and perhaps the example notebook. This would catch any issues with the example code, and make sure it stays up-to-date.
Some of the test (e.g. test/orbit.jl) check for things like misspecified parameters via try-catch, but only check that any error is thrown. These could be made more explicit by testing for the particular error that is expected.

General:

It might be worth reworking the plotting functions in the Recipes framework from Plots.jl. It would remove the Plots dependency (which is historically a fairly heavy-weight one), and allow more flexibility for the user. Also, PlotTrajectory() does not display in Pluto notebooks, which would likely be fixed.
I think there are some dependencies that could be dropped. For example, it doesn't look like Zygote or SciMLSensitivity are explicitly being used in any functions within the module. Others are BenchmarkTools, Enzyme, and ForwardDiff. It looks like Distributions is only used within the testing suite, which can have it's own Project.toml (like the docs).

Hopefully this is helpful! @tomkimpson please let me know if you need clarification on anything.

tomkimpson commented 1 year ago

Thanks @langfzac for the very helpful comments. I have opened a PR https://github.com/tomkimpson/RelativisticDynamics.jl/pull/42 and will ping you once all comments have been addressed

tomkimpson commented 1 year ago

@langfzac Thanks for all your comments, they were very useful! I have addressed each of them in turn here: https://github.com/tomkimpson/RelativisticDynamics.jl/pull/42#issuecomment-1588472777

langfzac commented 1 year ago

After merging tomkimpson/RelativisticDynamics.jl#42, all of my comments have been addressed.

tomkimpson commented 1 year ago

https://github.com/tomkimpson/RelativisticDynamics.jl/pull/42 is now merged.

All comments from both reviewers have now been addressed.

The only comment that has not is @duetosymmetry recommendation to change the package name. Having thought about it, I think RelativisticDynamics.jl is ok since the context around application to MPD is clear from the documentation and the JOSS paper. The subtitle is "Relativistic Spin-Orbital Dynamics in Julia" which I think also makes the application explicit.

If the reviewers disagree, I am happy to take to other naming suggestions - I am struggling to think of something succinct that follows the Julia package naming guidelines. Perhaps just RelativisticSpinOrbitalDynamics.jl?

If we can agree on an appropriate package name here I will then implement the change.

Thanks all.

tomkimpson commented 1 year ago

Quick ping for @dfm @duetosymmetry @langfzac just in case the above comment was missed

dfm commented 1 year ago

Thanks for the ping @tomkimpson! And thanks @langfzac for your constructive and efficient review!!

@duetosymmetry — Thanks for your review and feedback so far! It looks like @tomkimpson has made significant changes in response to your comments (https://github.com/tomkimpson/RelativisticDynamics.jl/pull/41), and has a remaining question. Please take a look as soon as you have a chance to see if you have more suggestions and if you can check off anymore checklist items. Thanks!!

dfm commented 1 year ago

Pinging @duetosymmetry again for feedback on the current state of the review here. Thanks!

arfon commented 1 year ago

@dfm – seems like this review has gone a little stale?

dfm commented 1 year ago

@arfon — thanks for checking in! There are several related email threads going in the background, but unfortunately it does look like we're going to need to assign a new reviewer (again!) because I'm not hearing back from @duetosymmetry on here or over email.

arfon commented 1 year ago

@dfm – these are the most similar historical papers:

DynamicalBilliards.jl: An easy-to-use, modular and extendable Julia package for Dynamical Billiard systems in two dimensions. Submitting author: @Datseris Handling editor: @kyleniemeyer (Active) Reviewers: @ahwillia Similarity score: 0.8195286949018195

DynamicalSystems.jl: A Julia software library for chaos and nonlinear dynamics Submitting author: @Datseris Handling editor: @kyleniemeyer (Active) Reviewers: @dhhagan Similarity score: 0.8132166880827265

NuclearToolkit.jl: A Julia package for nuclear structure calculations Submitting author: @SotaYoshida Handling editor: @rkurchin (Active) Reviewers: @mdavezac, @villaa Similarity score: 0.8131710416069782

QuasinormalModes.jl: A Julia package for computing discrete eigenvalues of second order ODEs Submitting author: @lucass-carneiro Handling editor: @pdebuyl (Active) Reviewers: @JamieBamber, @cescalara Similarity score: 0.810427232362389

iharm3D: Vectorized General Relativistic Magnetohydrodynamics Submitting author: @bprather Handling editor: @eloisabentivegna (Active) Reviewers: @bgiacoma, @cpalenzuela Similarity score: 0.8103136564020536

dfm commented 1 year ago

@editorialbot add @tamasgal as reviewer

Many thanks @tamasgal for agreeing to step in and pick up as a reviewer!! Thanks also to @tomkimpson for your patience with this process.

@tamasgal — You can comment @editorialbot generate my checklist on this thread to get your checklist and go from there. You'll find more info in the first comment on this thread, and this comment that I left near the start. Please let me know if you have any questions as we go!

editorialbot commented 1 year ago

@tamasgal added to the reviewers list!

dfm commented 1 year ago

@editorialbot remove @farr from reviewers

editorialbot commented 1 year ago

@farr removed from the reviewers list!

dfm commented 1 year ago

@editorialbot remove @duetosymmetry from reviewers

editorialbot commented 1 year ago

@duetosymmetry removed from the reviewers list!

tamasgal commented 1 year ago

Review checklist for @tamasgal

Conflict of interest

[x] 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

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the https://github.com/tomkimpson/RelativisticDynamics.jl?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@tomkimpson) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
[x] Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
[x] Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
[x] Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] 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

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
[x] 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

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] 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, who the target audience is, and its relation to other work?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] 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?

tamasgal commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

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

tamasgal commented 1 year ago

The package name RelativisticDynamics is fine in my opinion, there is no need to waste time on that ;) I was able to install the package without any issues and ran the test suite without any failing tests. Both notebooks (the demo and the one to reproduce the JOSS paper figures) executed fine in Jupyter and reproduced the results.

I opened a tiny issue regarding the compat specifications for dependencies and Julia itself: https://github.com/tomkimpson/RelativisticDynamics.jl/issues/56

Furthermore, I also suggest to add a simple CONTRIBUTING.md to the repository (edit: after reading the paper, the last paragraph of "statement of need" could be boldly copied into the contribution guideline ;)

I am now waiting for the latest PDF to be generated.

tamasgal commented 1 year ago

The paper is well-written and concise and the software package is a nice addition to JOSS, filling a niche gap in the field of relativistic spin-orbital dynamics which gained a lot of attention in the past years. Other than the two points above, I don't have any objections.

tomkimpson commented 1 year ago

Hey @tamasgal , thanks for the review, it's very appreciated! I have addressed all your comments in the above PR.

I'll merge if all looks good to you?

tamasgal commented 1 year ago

Perfect, let's move on! ;)

tomkimpson commented 1 year ago

@dfm all reviewer comments have now been addressed.

dfm commented 1 year ago

@tamasgal — I see that you've gone through all your checklist items - can you confirm that you're happy to sign off on acceptance? Thanks for your quick and thorough review!!

tamasgal commented 1 year ago

Yes, I am happy to sign off on acceptance, everything is fine from my side :)

dfm commented 1 year ago

@langfzac, @tamasgal — Thanks for your thorough and constructive reviews!!

@tomkimpson — I've opened a small PR with some minor edits to the manuscript, please take a look and merge or let me know what you think.

Once you've done that:

Take one last read through the manuscript to make sure that you're happy with it (it's harder to make changes later!), especially the author name and affiliation. I've taken a pass and it looks good to me!
Increment the version number of the software and report that version number back here.
Create an archived release of that version of the software (using Zenodo or something similar). Please make sure that the metadata (title and author list) exactly match the paper. Then report the DOI of the release back to this thread.

openjournals / joss-reviews