[REVIEW]: NBI: A library for Nystrom Boundary Integral calculations

editorialbot commented 12 months ago

Submitting author: !--author-handle-->@mjcarley@vissarion<!--end-editor-- Reviewers: @mscroggs, @bonh Archive: 10.5281/zenodo.12819049

Status

Status badge code:

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

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

@mscroggs & @bonh, 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 @vissarion 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 @mscroggs

📝 Checklist for @bonh

editorialbot commented 12 months 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 12 months ago

Software report:

github.com/AlDanial/cloc v 1.88  T=0.18 s (665.9 files/s, 290407.6 lines/s)
--------------------------------------------------------------------------------
Language                      files          blank        comment           code
--------------------------------------------------------------------------------
Bourne Shell                      4           2772           4257          16021
C                                24           1930           1789           7171
HTML                             27            197             81           5683
GLSL                              3              9             58           4748
CSS                               3            322             46           1621
m4                                8            167             37           1393
JavaScript                       10             97            148            835
C/C++ Header                      4            146            227            560
Bourne Again Shell               10             86            114            404
Markdown                          3             63              0            265
TeX                               3             26              4            214
MATLAB                           11             47              0            156
make                              5             20              1             84
SVG                               1              0              0             26
JSON                              1              0              0             23
Lisp                              1              1              0             23
YAML                              1              1              4             18
--------------------------------------------------------------------------------
SUM:                            119           5884           6766          39245
--------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository

editorialbot commented 12 months ago

Wordcount for paper.md is 912

editorialbot commented 12 months ago

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

OK DOIs

- 10.1016/j.jcp.2013.05.048 is OK
- 10.1016/j.jcpx.2021.100092 is OK
- 10.1016/b978-0-08-044371-3.x5000-5 is OK
- 10.1137/S1064827501399705 is OK
- 10.1121/1.3021297 is OK
- 10.21105/joss.02879 is OK
- 10.1002/nme.2579 is OK
- 10.1016/j.camwa.2009.10.027 is OK

MISSING DOIs

- None

INVALID DOIs

- None

editorialbot commented 12 months ago

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

mscroggs commented 12 months ago

Review checklist for @mscroggs

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/mjcarley/nbi/?
[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 (@mjcarley) 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?

vissarion commented 11 months ago

Hi, @mscroggs, @bonh could you please provide us with an update on the progress of this review?

bonh commented 11 months ago

Sry, I'll be on it end of the week!

bonh commented 10 months ago

Review checklist for @bonh

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/mjcarley/nbi/?
[x] License: Does the repository contain a plain-text LICENSE or COPYING file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@mjcarley) 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?

bonh commented 10 months ago

@vissarion The repo does not contain a LICENSE file but a COPYING file. The checklist specifically asks for the former but github seems to be okay with the latter, too. Please check the issue.

bonh commented 10 months ago

Right now I'm assessing the "Substantial scholarly effort" as requested by guidelines:

Age of software (is this a well-established software project) / length of commit history. not a problem, only a few months old, right?
Number of commits. a lot of work has been done on the code
Number of authors. only one, but that's alright, I guess
Total lines of code (LOC). a lot of code
Whether the software has already been cited in academic papers. no~~, right?~~ see https://github.com/mjcarley/nbi/issues/3
Whether the software is sufficiently useful that it is likely to be cited by your peer group. depends what the difference is to https://github.com/fastalgorithms/fmm3dbie, see https://github.com/mjcarley/nbi/issues/2

bonh commented 10 months ago

@editorialbot generate pdf

editorialbot commented 10 months ago

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

bonh commented 10 months ago

I have trouble compiling the package or dependencies, see https://github.com/mjcarley/sqt/issues/1.

mjcarley commented 10 months ago

I have included the four dependencies mentioned as submodules and updated README.md to include a note on using git clone --recursive when downloading the code.

bonh commented 10 months ago

To optimize my time effort I will pause my review until @mjcarley has decided on https://github.com/mjcarley/nbi/issues/5 :smile:.

vissarion commented 10 months ago

Hi @mscroggs are you still available to review this JOSS paper?

vissarion commented 10 months ago

Hi @mjcarley it seems that one reviewer is waiting for your feedback in https://github.com/mjcarley/nbi/issues/5 I see that you have provided a solution to the dependencies issue but please resolve https://github.com/mjcarley/nbi/issues/5 by agreeing on a solution with @bonh so that we can move on with the review. Thanks!

mscroggs commented 10 months ago

Hi @mscroggs are you still available to review this JOSS paper?

Hi. Yes, sorry I've been very slow with this review. I'll get on it this week.

bonh commented 9 months ago

I'm still not able to install the dependency blaswrap from @mjcarley, see https://github.com/mjcarley/blaswrap/issues/1.

All the projects make heavy use of automake, which I'm familiar with but not an expert by far. So I really hope that it's not my own stupidity.

Just to make sure: @mjcarley , have you tried installing your software packages on a different pc or platform yet? Or am I the first?

bonh commented 9 months ago

WIth the help of @mjcarley I was able to install the dependency blaswrap, thanks! However, nbi is still not running for me.

@mjcarley I'm really sorry but I feel like a beta tester here. To make sure that it's not just my own incompetence I'd appreciate if you'd confirm that someone else already tried to install and use your software. @vissarion Is that reasonable?

mjcarley commented 9 months ago

I am really sorry about this. Somebody else has installed the software, but they were working on a system similar to the one I developed it on, so the autoconf defaults were the same.

bonh commented 9 months ago

Good to know! :+1:

Is your system/platform that special? I tried installing it with a vanilla Ubuntu 20.

Perhaps it would help if you could use a common distribution in a virtual environment, install the dependencies via conda or so and install and run your software there?

mjcarley commented 9 months ago

It's a standard up to date Slackware installation. I can get access to a different (standard) system and will try installing there out of the box.

bonh commented 9 months ago

Yes, that sounds good! To make sure that it's not my fault...

vissarion commented 9 months ago

Thanks @mjcarley that would be great. You may consider using github actions (that have some standard Ubuntu environments) This will have the advantage that we will know if the code is still installable in future changes. Of course this is not mandatory but just a suggestion.

mjcarley commented 8 months ago

I have gone through an installation on an otherwise absolutely clean cloud-based system. Installation is carried out using a script containing (the environment variables used reflect the details of installation in my personal account under .../Codes/... ):

rm -rf nbi blaswrap git clone https://github.com/mjcarley/blaswrap git clone --recursive https://github.com/mjcarley/nbi cd blaswrap . autogen.sh ./configure --prefix=$HOME/Codes --with-blas=openblas \ CPPFLAGS=-I$HOME/Codes/include LDFLAGS=-L$HOME/Codes/lib make make install cd .. cd nbi . autogen.sh ./configure --prefix=$HOME/Codes \ CPPFLAGS=-I$HOME/Codes/include LDFLAGS="-L$HOME/Codes/lib -L$HOME/Codes/lib64" make

vissarion commented 8 months ago

Thanks! What OS/version are you using?

mjcarley commented 8 months ago

Release information says:

CentOS Linux release 7.8.2003 (Core)

vissarion commented 8 months ago

@bohn does the above instructions given by @mjcarley provide any help in installing the package?

vissarion commented 8 months ago

Hi, @mscroggs do you have any news on this review? It seems that there is an issue in installing the package (including the dependencies). Did you experience any difficulties during installation?

vissarion commented 7 months ago

Hi reviewers, @mscroggs @bonh! Do you have any updates on this review?

mscroggs commented 7 months ago

Sorry for being really slow on this. Some comments from me:

State of the field: I'd like to see a little more in the paper saying what NBI offers that existing software does not.
Substantial scholarly effort: Once you've added to State of the field, I think this will be clearer
Statement of need (documentation): There currently isn't a statement of need in the repo documentation. I recommend copying the into of the paper into the README.
Installation (on Ubuntu 22.04)
- Following the install instructions in the repo told me to install https://github.com/mjcarley/blaswrap. When doing make in blaswrap, I get some cryptic errors about libopenblas.
- Following the instructions in the comment about got further but then gave a very cryptic error when running make in nbi
- If you were able to provide a Docker image or similar with the dependencies and software installed, I think this would be very appreciated by users.
Automated tests: There are some instructions to run tests in the examples folder, which I'll try running after I've managed to install. It would be great if you could get GitHub Actions to automatically run these.
Community guidelines: I couldn't find this. You're welcome to borrow from CONTRIBUTING.md from my software Symfem (with no attribution necessary). I'd also recommend adding the Contributor Covenant code of conduct for contributors.
References: Looks like a couple DOIs are missing, otherwise these look fine

mjcarley commented 6 months ago

I've added the community guidelines and code of conduct.

The references that have a doi now have them in the reference list.

Can you say what the "cryptic errors" were when you installed blaswrap and nbi?

bonh commented 6 months ago

If I try to install blaswrap I get: undefined reference to `dtrtrs_'

vissarion commented 5 months ago

If I try to install blaswrap I get: undefined reference to `dtrtrs_'

I had the same issue (ubuntu 22, gcc-12.2.0), then deleted the repository, clone it again and run all steps from scratch. Now it built without successfully.

vissarion commented 5 months ago

Can you say what the "cryptic errors" were when you installed blaswrap and nbi?

@mscroggs Could you please reply to this question?

mjcarley commented 5 months ago

I have taken the suggestion of building a Docker image which is now available on the github repository. The image is built with two of the test cases run, and the other tests can be run in a shell in the Docker container. There is a corresponding update to the README.md, with information about running the examples, or other computations.

I have also updated the statement of features of the code to note the use of the CST approach of Kulfan to generate quite general "aerodynamic" geometries.

mscroggs commented 5 months ago

Can you say what the "cryptic errors" were when you installed blaswrap and nbi?

Sorry completely missed this question when you posted it. I think the useful line of the long error is:

/usr/bin/ld: blaswrap-tests.o: undefined reference to symbol 'dtrtrs_'

danielskatz commented 4 months ago

👋 @vissarion, @mscroggs, @bonh - it looks like this review is stalled. Is there anything I can do as track editor to help?

vissarion commented 4 months ago

@danielskatz thanks! There are several parts in this review that needs an update.

There is a problem for one reviewer (@mscroggs) in installing the package. See https://github.com/openjournals/joss-reviews/issues/5972#issuecomment-2076766108 that remains unaddressed. @mjcarley could you please reply?
The are two issues with a proposed solution by the author https://github.com/mjcarley/nbi/issues/2 and https://github.com/mjcarley/nbi/issues/5 but the reviewer (@bonh) didn't reply yet. @bohn could you please update the issue and if you find it fixed could you please close it as completed? @bonh is there something else that holds you back?

mjcarley commented 4 months ago

The problem in installing the package has been resolved and, as suggested by the reviewer, there is now a Docker image for the code, which can be downloaded from the github page.

vissarion commented 4 months ago

@mscroggs are you still getting the installation error? Did you manage to use the docker image to install the package?

danielskatz commented 4 months ago

👋 @mscroggs - I just wanted to check on if you are still able to work on this? (As track editor, I try to check on reviews in the CSISM track where no progress has been recorded in a 2-week period.)

mscroggs commented 4 months ago

Sorry, I was on leave with little internet for 2 weeks then away at a conference. I was able to run the software in the docker image (thanks for providing this). Updating my checklist now

mscroggs commented 4 months ago

@editorialbot generate pdf

editorialbot commented 4 months ago

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

vissarion commented 4 months ago

@bonh could you please update us on your review progress? Last time you had issues in compiling the source code. Did you manage to try the docker image? That approach was successful for @mscroggs who was also experiencing installation issues.

bonh commented 3 months ago

@editorialbot generate pdf

editorialbot commented 3 months ago

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

openjournals / joss-reviews