-
-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[REVIEW]: APECSS: A software library for cavitation bubble dynamics and acoustic emissions #5435
Comments
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:
|
|
Wordcount for |
|
👋 Hi @fabiandenner @jmansour @svchb this is where the actual review takes place. Thanks! |
Thanks @kyleniemeyer. Thanks @jmansour and @svchb for accepting the invitation to review our work. Please don't hesitate to let me know if you have any questions or suggestions. |
Review checklist for @svchbConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
@fabiandenner Your paper is lacking a section in which you compare APECSS to other software tools that can be used for bubble dynamics. I must add personally I am not aware of other software packages in this area. |
@kyleniemeyer It seems like @jmansour is not participating. |
Hi all. Apologies, this completely fell off the radar and I failed to notice the Github notifications until now. |
@svchb I agree, a direct comparison of APECSS to similar software tools would be a nice addition. On multiple occasions, also just before submitting to JOSS as we anticipated this question, we tried our best to identify open-source tools with similar capabilities. However, we also couldn't find any. We validated APECSS thoroughly, especially in our Physics of Fluids paper published earlier this year, and this can also be checked with the examples provided in the repository (we mention the literature case it should reproduce in the README.md accompanying each example). In terms of performance, we could conduct a comparison to an in-house Python implementation, using SciPy functions (with the same Runge-Kutta method) to solve the governing ODE, for a few examples. I'm not sure how representative such an implementation would be, but it could at least give a rough orientation about the performance benefit. What do you think? |
@fabiandenner I had not yet time to check your code or examples. Anyway I am not sure how representative that would be as performance of Python based solvers largely depends on avoiding native Python code execution of computational intensive parts. |
@svchb I agree, our Python scripts are perhaps best described as a simple implementation with off-the-shelf components, not a sophisticated or performance-optimized implementation. |
@fabiandenner Please add how to run an example to your Readme.md. |
@svchb Yes it is supposed to perform time steps. The README.md of this test case provides an example of the execution command (that corresponds to reproducing the references literature result). I think what's missing in your execution are the excitation frequency and amplitude, and the end time of the simulation. I'll expand the Quick Start section of the main README.md file to include the additional steps to running one of the examples. The help arguments is a nice idea, thanks for this suggestion. I'll add this. Also, I'll have a look into the error messages. |
@fabiandenner Oh I didn't read the README.md in the examples folder. Maybe point to these files more explicitly from the repository README.md. |
@svchb I just committed some small changes. I expanded the Quick Start Guide in the primary README.md and moved it further up in the text, so that it is better visible. Also, -h now provides some basic command line help. |
Review checklist for @jmansourConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
@editorialbot generate pdf |
Some comments:
Tests:
Besides this testing aspect I am satisfied with the paper, documentation and code. |
Done! archive is now 10.5281/zenodo.8043400 |
@editorialbot set v1.4 as version |
Done! version is now v1.4 |
@editorialbot check references |
|
@fabiandenner can you check on those potentially missing DOIs in the paper? |
@kyleniemeyer One of the missing DOIs, for the book Underwater Explosions is spot on, well done editorialbot. I've added it to the bib file. Regarding the bot-suggested DOI, for the report The Collapse of a Spherical Cavity in a Compressible Liquid, it does not seem to work. However, the Cal Tech address in the reference is supposedly a persistent identifier provided by Cal Tech. |
@editorialbot generate pdf |
Great! looks good to me. |
@editorialbot recommend-accept |
|
|
👋 @openjournals/pe-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#4315, then you can now move forward with accepting the submission by compiling again with the command |
Looks all good to me. |
@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. |
🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦 |
🐘🐘🐘 👉 Toot 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... |
@kyleniemeyer Thanks for wrapping this up and thanks to @svchb and @jmansour for taking the time to review our paper and code. I hope other journals take notice of the excellent workflow JOSS is running, very good work! |
Congratulations @fabiandenner on your article's publication in JOSS! If you haven't already, please sign up as a reviewer so you can help us with submissions in the future: https://reviewers.joss.theoj.org/join |
🎉🎉🎉 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:
|
Submitting author: @fabiandenner (Fabian Denner)
Repository: https://github.com/polycfd/apecss
Branch with paper.md (empty if default branch): joss
Version: v1.4
Editor: @kyleniemeyer
Reviewers: @jmansour, @svchb
Archive: 10.5281/zenodo.8043400
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
@jmansour & @svchb, 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 @kyleniemeyer 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 @svchb
📝 Checklist for @jmansour
The text was updated successfully, but these errors were encountered: