-
-
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]: AlignSAR: An open-source toolbox of SAR benchmark dataset creation for machine learning applications #7532
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: |
👋🏼 @LC-SAR, @philippemiron, and @shahchiragh this is the review thread for the paper. All of our communications will happen here from now on. As a reviewer, the first step is to create a checklist for your review by entering
as the top of a new comment in this thread. These checklists contain the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. The first comment in this thread also contains 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 We aim for reviews to be completed within about 2-4 weeks. Please let me know if any 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 (@rwegener2) if you have any questions/concerns. |
Review checklist for @philippemironConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
Hey @philippemiron and @shahchiragh, just a friendly ping about this review. @shahchiragh, the first step is to generate your checklist using If you have any questions don't hesitate to reach out! |
Review checklist for @shahchiraghConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
@editorialbot check references |
|
Hi @rwegener2 , I’ve completed an initial round of the review. However, a few checks, specifically related to installation and documentation, are still pending as they depend on the software installation process. I will revisit these once the authors address the comments I’ve summarized in the repo issue. Additionally, there are a few aspects related to the software paper, such as the "Quality of Writing" and "References," that I would like to bring to the authors' attention for further refinement of the paper. Please feel free to reach out for any questions or comments. Thanks! |
Thanks for the thoughtful comments @shahchiragh! Sounds good for next steps. |
Thank you @rwegener2, @philippemiron and @shahchiragh, we have updated the AlignSAR repo based on your valuable suggestions. Please check my response to your comments in this channel, and our updated repo. |
Hi there everyone, In reading over the status of this paper review I'm noticing some themes from both reviewers. It seems that installation and the ability to find and run tests have been difficult for both reviewers. It appears that some of this information is available, but was perhaps hard to find. @philippemiron and @shahchiragh, if either of you have suggestions for how the authors could reorganize to make the tests easier to run or locate those could be useful pieces of feedback for the authors. As for installation, that is an important point for the authors to address. It also hinders the ability of a reviewer to run evaluate the testing and functionality aspects of the review. @LC-SAR if there is any information (Ex. operating systems and versions) that the reviewers can provide to help you debug these installation problems please feel free to ask follow up questions. |
Thank you @rwegener2 for your comments and suggestions. We have updated the repo based on two reviewers' suggestions. For theirlast comments on docker installation, we tested internally again with three different operating systems/software, all work from our side. We await two reviewers' further responses, and then we will update the information related to e.g. operating systems and versions on the github repo. Thank you |
Hi @LC-SAR. I understand that it can be difficult to test and account for every possible architecture. Still, it is important to remember that our reviewers are volunteers and debugging an installation can be quite time intensive, especially when they are new to a software. I also went through and tested the installation. It was successful when I installed on my Mac (Sonoma 14.6.1). I provided some suggestions for improving the installation instructions in a repository issue. Hopefully between myself and the reviewers we can help ensure the AlignSAR installation and the instructions are as clear and accessible to as many people as possible. It seems like perhaps the Mac installation is operating well, but @philippemiron and @shahchirag are operating on Linux systems (is that correct?). Can you both please provide your linux flavors and versions? I see a few possible routes forward:
I think this is a great software for the community and I'd love to see it keep moving forward. Let me know what you think the best next step is. |
Hi @rwegener2, apologies for the delay in getting back and troubleshooting this issue, as I understand it may have added extra work for the authors. I was able to take out some time to thoroughly troubleshoot the installation steps and identified the necessary modifications to the Dockerfile that resolved the issues. I’m using macOS Sequoia 15.2 and I had to adjust my Python version to 2.7.18. Even after the Python version change, I ran into several installation errors, but with some additional tweaks, I was able to get everything working. I've shared these changes with @LC-SAR in Issue 3 as a reference for troubleshooting, especially for users with different setups. I'll now proceed with the remaining items in the review, but if there's anything else you’d like me to check, please don’t hesitate to reach out. |
Review checklist for @shahchiraghConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
@editorialbot check references |
|
Sounds good, @shahchiragh. It is very kind of you to take the time to dig into the errors. Thanks so much for your thoroughness in this review! |
Dear @shahchiragh, thanks for taking the time to test and offer suggestions on improving our dockerfile. We tested the dockerfile you suggested on mac OS successfully, and included it in our github repo, see https://github.com/AlignSAR/alignSAR/blob/main/misc/Dockerfile1. In Dockerfile1, we only modified the version of fftw from 3.3.10 to 3.2.2, as the higher version of fftw is not compatible with Doris software. We also supplemented the instruction in https://github.com/AlignSAR/alignSAR/blob/main/README.md, explaining the 'Dockerfile works well with three different operating systems/docker versions: macOS Ventura 13.7.3 (docker version: version 4.37.2), ubuntu 20.04.6 LTS (docker version: 24.0.5) and Ubuntu 20.04.1 (docker version: 24.0.7). If you encounter version-related errors or missing Python packages while running this Dockerfile on macOS Sequoia 15.2, please use Dockerfile1 in the folder 'misc' instead.' Looking forward to your further suggestions. |
Submitting author: @LC-SAR (Ling Chang)
Repository: https://github.com/AlignSAR/alignSAR
Branch with paper.md (empty if default branch): main
Version: v1.1
Editor: @rwegener2
Reviewers: @philippemiron, @shahchiragh
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
@philippemiron & @shahchiragh, 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 @rwegener2 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 @philippemiron
📝 Checklist for @shahchiragh
The text was updated successfully, but these errors were encountered: