-
-
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]: emg3d: A multigrid solver for 3D electromagnetic diffusion. #1463
Comments
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @akelbert, it looks like you're currently assigned as the reviewer for this paper 🎉. ⭐ Important ⭐ If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿 To fix this do the following two things:
For a list of things I can do to help you, just type:
|
|
@akelbert @emersodb @lukeolson 👋 Welcome and thanks for agreeing to review! The comments from @whedon above outline the review process, which takes place in this thread (possibly with issues filed in the emg3d repository). I'll be watching this thread if you have any questions. |
Great to see the review started! I repeat a question here that I asked in the original issue (#1431 (comment)): "I saw one typo and some of the reference-bracketing in the compiled version did not turn out as I intended. Is there a way to edit them now or shall I wait for the reviews first?" |
You can make changes in the paper.md and paper.bib - after you have done so, enter a new comment here containing |
@whedon generate pdf |
|
I pushed a revised version. The only thing I did was changing things a little to take into account that citations in JOSS seem to always have brackets (). So I avoided having (()), and tried to make sentences that do not have a (citation) in its flow. Nothing change with regards to the context. |
Not a super important issue, but when doing the pip install, the version number installed was emg3d-0.6.1 rather than 0.5.0 as indicated in the version. Should we be forcing an install of 0.5.0 for the review? |
Since I submitted the manuscript I worked on it and released v0.6.0 and v0.6.1. I don't know what is the normal procedure in JOSS, if using the most recent version to final publication date or the version when it was submitted. Asking @jedbrown ? |
It's typical to review the most recent release or maintenance/stable release. When revisions occur as a result of review (also typical) then the final archived version will be tagged post-review (could be v0.6.2, for example). Assuming you would like v0.6.1 to be reviewed, I'll update that now. |
@whedon set v0.6.1 as version |
OK. v0.6.1 is the version. |
A minor issue that I ran into while working through the 1c_3D_triaxial_SimPEG example is that, as I use pip as my python package manager, pymatsolver does not automatically include Pardiso since MKL is not packaged within pip distributions as it is in Anaconda. So I needed to do a pip install MKL before the example could be run using Pardiso |
In the examples repository, it appears that the folder SEG-EAGE/' is missing from the data/ folder. Thus, when I try to run the 2a_SEG-EAGE_3D-Salt-Model.ipynb notebook, it yields that exception FileNotFoundError: [Errno 2] No such file or directory: './data/SEG-EAGE/SALTF.ZIP' |
|
|
Ah, apologies on the data-side. I missed that comment in the notebook! |
No problem at all. I just added the following sentence under requirements in the notebook |
|
I may have missed this in the examples or elsewhere in the repository, so please correct me if I'm wrong, but I didn't notice if there were any examples exhibiting optimal scaling for the MG solver. It would be beneficial, I think, to add an example showing linear scaling of the solver, even for just a very simple setup. Otherwise, I think the examples do a good job of verifying the algorithm against existing solvers. |
That is a good and timely question @emersodb. Since version 0.6.2 (so two days ago) I had a CPU & RAM section in the docs, and a notebook to estimate memory. Given your question I expanded this section and also added a notebook to estimate runtime.
Figures |
At this time, I believe that my review is complete and would recommend publication of the emg3d submission. |
Thanks for your time, input, and feedback @emersodb |
OK. 10.5281/zenodo.3339421 is the archive. |
@whedon accept |
|
Check final proof 👉 openjournals/joss-papers#839 If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#839, then you can now move forward with accepting the submission by compiling again with the flag
|
@openjournals/joss-eics This paper is accepted; over to you. |
Thanks @jedbrown , was a pleasure to work with you! |
Likewise! Thanks for your patient and careful work, and thanks to our reviewers. |
@prisae - please change "A diffusive problem remains, which resulting system of equations is given in the frequency domain by" to "A diffusive problem remains, which has the resulting system of equations given in the frequency domain by" "It currently uses mostly" should be "It currently mostly uses" or "It currently primarily uses" "the multigrid CSEM codes of these publications" should be "the multigrid CSEM codes discussed in these publications" |
@danielskatz , does that imply that I'll have to make a new release and mint another Zenodo DOI with it? I am off on vacation right now, I'll do it when I am back on Monday. |
no, the paper source doesn't need to be in the repo or if it is, it doesn't need to be up to date - we care about the software repo and the pdf of the paper in terms of archiving |
@whedon generate pdf |
|
I made the corrections @danielskatz |
@whedon accept |
|
Check final proof 👉 openjournals/joss-papers#852 If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#852, then you can now move forward with accepting the submission by compiling again with the flag
|
@whedon accept deposit=true |
|
🐦🐦🐦 👉 Tweet 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 to @akelbert & @emersodb & @lukeolson for reviewing and @jedbrown for editing! |
🎉🎉🎉 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! 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: @prisae (Dieter Werthmüller)
Repository: https://github.com/empymod/emg3d
Version: v0.7.1
Editor: @jedbrown
Reviewer: @akelbert, @emersodb, @lukeolson
Archive: 10.5281/zenodo.3339421
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
@akelbert & @emersodb & @lukeolson, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @jedbrown know.
✨ Please try and complete your review in the next two weeks ✨
Review checklist for @akelbert
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
paper.md
file include a list of authors with their affiliations?Review checklist for @emersodb
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
paper.md
file include a list of authors with their affiliations?Review checklist for @lukeolson
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
paper.md
file include a list of authors with their affiliations?The text was updated successfully, but these errors were encountered: