-
-
Notifications
You must be signed in to change notification settings - Fork 38
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]: DisCoTec: Distributed higher-dimensional HPC simulations with the sparse grid combination technique #7018
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:
|
|
Software report:
Commit count by author:
|
Paper file info: 📄 Wordcount for ✅ The paper includes a |
License info: 🟡 License found: |
Review checklist for @EmilyBourneConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
@EmilyBourne and @jakelangham - Thanks for agreeing to review this submission. As you can see above, you each should use the command 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, reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention We aim for reviews to be completed within about 2-4 weeks. Please let me know if either of you require some more time. We can also use editorialbot (our bot) to set automatic reminders if you know you'll be away for a known period of time. Please feel free to ping me (@danielskatz) if you have any questions/concerns. |
@EmilyBourne - how is your review coming along? |
👋 @jakelangham - note that you need to use the command |
@danielskatz Sorry it's coming a little slowly. I wasn't too worried as Jake didn't seem to have started but I will try and get it done ASAP. |
@EmilyBourne - this certainly isn't urgent, but I try to ping on these after a couple of weeks just to make sure things have started and continue to move at some non-zero speed 🙂 |
Review checklist for @jakelanghamConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
@EmilyBourne - thanks for opening these issues |
👋 @jakelangham - how is your review coming along? |
@danielskatz, @freifrauvonbleifrei I took a proper look today. (Apologies if I'm late.) From what I've seen at so far, it looks like a well written and potentially useful library that seems entirely suitable for a JOSS paper. However, at the outset I'd like to echo @EmilyBourne's observation in SGpp/DisCoTec#129 about the lack of documentation. The README is nice and very welcome but I was expecting to see some sort of detailed guide for a newcomer about how to actually use the software. Without this, I cannot adequately review the library because I only have a partial understanding of what it does and what all its features are. Therefore, I think that major revisions are required in this direction before proceeding further. The JOSS reviewer guidelines are somewhat discretionary on what counts as sufficient documentation, but essentially what I'm looking for is to convince myself that a target user could get to grips with DisCoTec and use it to solve problems completely independently of input from the developers and I don't think this is currently the case. |
Thanks @jakelangham - I think your comment is correct about what JOSS is looking for
|
That's correct, and they're not all trivial so imagine it may take some time to work through them all. |
@danielskatz I am in a similar place to @jakelangham . I was waiting for the documentation to be addressed so I could try to use the software on some of my own problems to ensure that the functionality is ok and that the documentation is clear enough to make this possible.
Without a Library API it is difficult to have an overview of the functionalities. We can check the ones used in the tutorial but I imagine that these are not the only functions available. I am also waiting for SGpp/DisCoTec#133 to be merged before checking the associated boxes in my review. |
👋 @freifrauvonbleifrei - Can you update us on your progress (and/or schedule) for responding to the issues from @jakelangham and @EmilyBourne? |
@danielskatz thank you for raising the uncomfortable questions! ;) Edit: The API Documentation is now linking correctly: https://discotec.readthedocs.io/en/latest/api/library_root.html The harder questions were arising for me once I started fine-grained documentation: I don't think I and the co-authors will be able to cover the full API in detail -- this is the curse of a multi-PhD-generational code (it's broad, and would need significant brainpower to re-understand every part). But I'd rather not throw out useful functionality. On the other hand, snippets of Doxygen are sprinkled throughout all parts of the code, making the current Doxygen overview quite messy to read. So my suggestion would be to focus on the classes that framework users would most likely interact with ( Please do let me know your thoughts and inner resistances -- quite possibly, there is a smarter way to go about it. |
@freifrauvonbleifrei I agree with this approach - it's best to focus on the core functionality. Even better if you can highlight the important parts of the API (or remove the unnecessary auto-generated boilerplate stuff). The review criteria for JOSS indicate that only the core functions/classes needed to be documented (though obviously more documentation is usually better). |
Thanks @freifrauvonbleifrei and @jakelangham. Let's go ahead unless @EmilyBourne has a different suggestion that we should discuss first. |
No this all sounds good to me |
@freifrauvonbleifrei - I just want to confirm that you are working on this... |
Sorry, I had forgotten to push my added documentation to here: SGpp/DisCoTec#133 I am currently trying to remove the worst documentation clutter and am on track to merge today :) |
Thanks. (and FYI, I may be off-line next week) |
@freifrauvonbleifrei - How is this going now? |
@danielskatz good question! I was hoping that now @EmilyBourne and @jakelangham would start their hands-ons, while I further refine the documentation (progress here https://github.com/SGpp/DisCoTec/tree/refine_documentation) according to the things that were already mentioned here: SGpp/DisCoTec#129 , and trying to make the Doxygen output more usable. probably issue 129 would also be a good spot for further docs-related feedback to give things to incorporate on a rolling basis ;) |
I have a couple of deadlines coming up soon so I haven't got back to this yet but I will get on it as soon as I have time (hopefully some time next week) |
👋 @EmilyBourne & @jakelangham - just checking with you on this again... |
@danielskatz I am just getting back to this but am still quite busy. My last deadline this year is 15th December. I have started looking through the docs again but I'm not sure I will have the chance to test out my own implementations before then. After the 15th though I should have enough time so I will make sure I am done with my review by the 20th. |
@jakelangham - it looks like you are close to done. Can you finish up fairly soon as well? Or are there things blocking you? |
Hi, @danielskatz, have I read back through the thread and hope there hasn't been a miscommunication here. I finished my review (in the sense of interrogating every bullet point) some time ago and was waiting for the points I raised to be addressed, after which I expected to be happy to recommend for publication. Specifically, I can see SGpp/DisCoTec#134, SGpp/DisCoTec#137, SGpp/DisCoTec#129. As far as I'm aware there has been some progress but they're still being worked on. |
Thanks @jakelangham. It's helpful to have this in one place, and I apologize if it seemed like I was blaming you; I was just trying to pull all the statuses together. 👋 @freifrauvonbleifrei - can you update us here on how the work on these open issues are going? |
@jakelangham thx for the overview! I had started working diffusely on the different questions but it really helps to see them as a (finite) list! :D |
@danielskatz I think that now we are on top of all the issues, but happy to know if there's more we can clarify / extend in the documentation. |
@editorialbot check references |
|
@editorialbot check references |
|
Thanks - I hope the reviewers can comment on if more is needed. |
Hi all, I am travelling (again) this week but should be able to get to review everything again before end-of-year holidays. |
@freifrauvonbleifrei @danielskatz I've looked through all the improvements and submitted a few suggestions but they are very minor in scope, so I probably don't have more to add here unless there are specific questions about things. In short, I'd be happy if the paper proceeded forward to publication. @freifrauvonbleifrei Two additional things that need doing beforehand: (1) check the references + DOIs (e.g. link on the last ref, [Verboncouer 2005] might need tidying up somehow); (2) all the improvements need merging into main (excepting the paper, which can live on its own branch, if preferred). |
Thanks @jakelangham! |
👋 @EmilyBourne - just a reminder on this, which you probably don't need...
|
Submitting author: @freifrauvonbleifrei (Theresa Pollinger)
Repository: https://github.com/SGpp/DisCoTec/
Branch with paper.md (empty if default branch): feature_joss_submission
Version: 1.0.0
Editor: @danielskatz
Reviewers: @EmilyBourne, @jakelangham
Archive: Pending
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
@EmilyBourne & @jakelangham, 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 @danielskatz 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 @EmilyBourne
📝 Checklist for @jakelangham
The text was updated successfully, but these errors were encountered: