joss-reviews
joss-reviews copied to clipboard
openjournals
[REVIEW]: cppTPSA/pyTPSA: a C++/Python package for truncated power series algebra
Submitting author: @zhanghe9704 (He Zhang) Repository: https://github.com/zhanghe9704/tpsa Branch with paper.md (empty if default branch): Version: v1.0.0 Editor: @pibion Reviewers: @dtabell, @chrisrogers1234 Archive: Pending
Status
Status badge code:
HTML: <a href="https://joss.theoj.org/papers/1fa756415c650531cdfd292233b4cc8d"><img src="https://joss.theoj.org/papers/1fa756415c650531cdfd292233b4cc8d/status.svg"></a>
Markdown: [](https://joss.theoj.org/papers/1fa756415c650531cdfd292233b4cc8d)
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
@dtabell & @chrisrogers1234, 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 @pibion 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
@dtabell, please create your checklist typing: @editorialbot generate my checklist
@chrisrogers1234, please create your checklist typing: @editorialbot generate my checklist
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
Software report:
github.com/AlDanial/cloc v 1.88 T=0.27 s (1160.3 files/s, 258622.3 lines/s)
--------------------------------------------------------------------------------
Language files blank comment code
--------------------------------------------------------------------------------
HTML 151 761 395 25217
C/C++ Header 4 3194 1160 14119
TeX 28 1100 241 7939
C++ 7 728 1379 4056
JavaScript 100 173 200 3195
XML 4 0 129 1954
CSS 4 361 51 1785
Markdown 3 133 0 200
Bourne Again Shell 1 31 9 141
SVG 3 1 1 129
make 2 10 0 33
DOS Batch 1 4 0 27
CMake 1 4 6 9
--------------------------------------------------------------------------------
SUM: 309 6500 3571 58804
--------------------------------------------------------------------------------
gitinspector failed to run statistical information for the repository
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):
OK DOIs
- 10.1063/1.41975 is OK
- 10.1023/A:1024467732637 is OK
- 10.1016/j.nima.2011.01.053 is OK
- 10.1016/s1076-5670(08)x7018-1 is OK
- 10.2172/812598 is OK
- 10.1016/j.nima.2005.11.109 is OK
- 10.2514/6.2018-0398 is OK
MISSING DOIs
- None
INVALID DOIs
- None
:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
@chrisrogers1234, could you try generating your review checklist and confirm that you can post on this thread?
@dtabell, are you still able to review? I know it's been a long time since you agreed to review.
@pibion I can post. However, I am full this week and next and unable to get started on this until w/c 17th October.
My apologies! Life has a way of intervening, but that is an excuse of no merit. I have put this on my weekend task list. Dan
On 6 Oct 2022, at 17:21, pibion @.***> wrote:
@dtabell https://github.com/dtabell, are you still able to review? I know it's been a long time since you agreed to review.
— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4818#issuecomment-1270835660, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACXTUHBB4N54IZNYXEBUBG3WB5NINANCNFSM6AAAAAAQ5Y3ILA. You are receiving this because you were mentioned.
@chrisrogers1234 thanks for confirming! Getting started on 17th October is fine, I just like to make sure people have access :)
@dtabell great to hear you're still on board!
@zhanghe9704
Could you advise how to build the tests? I tried doing something like gcc tests.cc -o test -L../ -ltpsa -lstdc++ -lm -std=c++14 and get loads of "undefined references" to Catch::
Many thanks!
@zhanghe9704 @pibion I attach referees comments.
Summary
The code seems to be interesting. However, it is very hard to penetrate what is doing what to which code block without some further documentation or overview. I was able to build and run the example but not the tests.
2022-10-17_reviewer-comments.txt
@chrisrogers1234 https://github.com/chrisrogers1234 Sorry for the delay. It takes some time for me to remember what I did since I haven't touched the code for about a year.
Turns out the Makefile for the tests is not included in the online repository. I attached it here.
You need to use cmake and make to compile the libs first. Then in the test folder, use make with Makefile to compile the tests. (The libs from the above step will be needed and will be found automatically.)
An alternative way is to compile the libs using the cbp project and then compile the test using the cbp project if you use CodeBlocks. I actually use CodeBlocks to develop the code. The cmake file and Makefiles were added and tested after the cbp projects.
Thanks for reviewing the submission.
Best regards, He
On Mon, Oct 17, 2022 at 4:59 AM chrisrogers1234 @.***> wrote:
@zhanghe9704 https://github.com/zhanghe9704
Could you advise how to build the tests? I tried doing something like gcc tests.cc -o test -L../ -ltpsa -lstdc++ -lm -std=c++14 and get loads of "undefined references" to Catch::. I note Catch is an external test package but it looks like you have copied some source code into your library. Should I build Catch as an external dependency or is it supposed to be built as part of the tests (and if so what is the recipe?). Can I build the tests from within cmake/make or is it like the examples?
Many thanks!
— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4818#issuecomment-1280519649, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB5QICT7X6OEJSVP3EFUASTWDUIOFANCNFSM6AAAAAAQ5Y3ILA . You are receiving this because you were mentioned.Message ID: @.***>
@zhanghe9704
Thanks! I will have a look, maybe later this week. Also I attached a few recommendations above. The code looks good, but I think I need a bit more documentation just to orient my brain to help with the review.
@zhanghe9704 would you be willing to add the makefile for the test compilation to the code repository? I don't think it attached (or perhaps I'm not seeing it) in this issue thread. And having it in the repository itself would be helpful.
@zhanghe9704 floating the request to add the makefile to your repo up to the top. Thank you!
The Makefile has been uploaded. Sorry for the delay.
On Mon, Oct 31, 2022 at 3:11 PM pibion @.***> wrote:
@zhanghe9704 https://github.com/zhanghe9704 floating the request to add the makefile to your repo up to the top. Thank you!
— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4818#issuecomment-1297547211, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB5QICV6NRZVW5WAHKFVWHLWGAKVVANCNFSM6AAAAAAQ5Y3ILA . You are receiving this because you were mentioned.Message ID: @.***>
@zhanghe9704 I'm having trouble compiling the libraries and binaries. I've tried with CodeBlocks, but it seems to be having trouble recognizing that the default compiler is gcc. I've tried opening and building cbp/tpsa_dll.cbp and cbp/tpsa_lib.cbp. In both cases I get the following error:
The compiler's setup is invalid, so Code::Blocks cannot find/run the compiler.
Probably the toolchain path within the compiler options is not setup correctly?!
Do you have a compiler installed?
Goto "Settings->Compiler...->Global compiler settings->unknown->Toolchain executables" and fix the compiler's setup
I suspect the issue here is that I don't actually have a gcc compiler installed that codeblocks can find, so I'll work on that.
In the meantime, I tried building the code in WSL, where I have a new-ish version of gcc installed (9.4.0). When I first ran cmake ., I got an error saying CMake couldn't find the C++ compiler. Per its suggestion I set an environment variable, CXX, pointing to my gcc. But then I get an error saying CMake couldn't compile a test program.
Are there any other environmental variables you set?
@zhanghe9704 I was able to compile the libraries and tests in WSL by installing g++ :D. However, when I try to run the tests executable, I get a segmentation fault in addition to the tests passing:
(base) aroberts@LAPTOP-74V3RK8A:/mnt/c/Users/canto/Repositories/tpsa/test$ ./tests
===============================================================================
All tests passed (21 assertions in 3 test cases)
Segmentation fault
Have you encountered this before?
Still working on Codeblocks, if you have any suggestions I'd be grateful!
@pibion Thanks for your work.
You can tell CodeBlocks where to find the compiler in "settings" - "compiler" - "Toolchain executables". Then right-click the project name, and choose "build option" to select which compiler you want to use to compile the project. "Ctr+F9" will compile the project. "Ctr+F11" will recompile all the files in the project.
I also have the "Segmentation fault" when running the tests in WSL. But I don't have this problem when I compile and run the tests on windows. I also don't have the segmentation fault when I use the lib either in C++ or in Python. So I suspect it is related to how Catch2 manages the memory in WSL. (The TPSA lib will allocate a big chunk of memory to save the TPS vectors in the beginning and release the memory when the program quilts. Maybe it has some conflicts with Catch2.) I just ignored it since all the tests were passed and there was no such error when using the lib.
Best regards, He
On Thu, Nov 3, 2022 at 1:16 PM pibion @.***> wrote:
@zhanghe9704 https://github.com/zhanghe9704 I was able to compile the libraries and tests in WSL by installing g++ :D. However, when I try to run the tests executable, I get a segmentation fault in addition to the tests passing:
(base) @.***:/mnt/c/Users/canto/Repositories/tpsa/test$ ./tests
All tests passed (21 assertions in 3 test cases)
Segmentation fault
Have you encountered this before?
Still working on Codeblocks, if you have any suggestions I'd be grateful!
— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4818#issuecomment-1302430091, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB5QICWVX4CV4ZV6VNX53CTWGPXQBANCNFSM6AAAAAAQ5Y3ILA . You are receiving this because you were mentioned.Message ID: @.***>
Hi @pibion - what's the status of this? I don't see any reviewer checklists which confuses me.
👋 @pibion - It looks like it's been another three weeks here with no activity that I can see. What are the next steps to continue the processing of this submission?
@danielskatz
Did you manage to address the documentation requests in this note?
https://github.com/openjournals/joss-reviews/files/9811088/2022-10-17_reviewer-comments.txt
@chrisrogers1234 - I'm the track editor here, so I don't think you meant this for me, but for the author, @zhanghe9704
As a side note, this would be easier to see and work with if it was not an attachment, and was instead a bunch of issues in the source respository, each including a tag to this review (ie a mention of openjournals/joss-reviews#4818) - this will create a link to this thread, and here, we'll be able see if each is still open or closed.
Actually, since I don't see review checklists, here, let me provide my version of instructions:
@dtabell and @chrisrogers1234 - Thanks for agreeing to review this submission. This is the review thread for the paper. All of our communications will happen here from now on.
As you can see above, you each should use the command @editorialbot generate my checklist to create your review checklist. @editorialbot commands need to be the first thing in a new comment.
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 openjournals/joss-reviews#4818 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.
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.
Review checklist for @chrisrogers1234
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/zhanghe9704/tpsa?
- [ ] 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 (@zhanghe9704) 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
- [ ] Installation: Does installation proceed as outlined in the documentation?
- [ ] Functionality: Have the functional claims of the software been confirmed?
- [ ] 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
- [ ] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
- [ ] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
- [ ] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
- [ ] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
- [ ] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
- [ ] 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
- [ ] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
- [ ] 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?
- [ ] State of the field: Do the authors describe how this software compares to other commonly-used packages?
- [ ] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
- [ ] 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?
@chrisrogers1234 - Sorry, I haven't worked on that. Unfortunately, I'm very busy now with two proposals due in early January. I don't think I'll have much time to work on this till the end of the year but I'll try. BTW, I'm new to this process and appreciate any instructions.
Thanks for all your help. Happy holiday!
On Fri, Dec 16, 2022 at 7:35 AM chrisrogers1234 @.***> wrote:
@danielskatz https://github.com/danielskatz
Did you manage to address the documentation requests in this note?
https://github.com/openjournals/joss-reviews/files/9811088/2022-10-17_reviewer-comments.txt
— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4818#issuecomment-1354702933, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB5QICSN3TMD5ABMIK4ARZTWNROX7ANCNFSM6AAAAAAQ5Y3ILA . You are receiving this because you were mentioned.Message ID: @.***>
Thanks @zhanghe9704
The checklist does not allow me to make comments or qualifications. However there are several notes in the referees report. I won't check anything else off until the issues in the referee report have been dealt with, if that's okay. Note that I tripped over on documentation and build so I didn't progress further until all of that has been sorted out. The other stuff might be okay, but I would like e.g. the documentation to be improved before I dig further.
Ideally, the process here is that reviewers find problems that keep them from checking off all the items on their checklist, and express those problems either as comments here (if very simple) or as issue or PRs in your repository. You try to work on them, or discuss why you think you don't need to. The editor (@pibion) moderates the discussion as needed to get to consensus, and makes decisions if there doesn't seem to be consensus. This process continues until the submission is acceptable, or until you decide to stop it, and the whole process ideally takes 4-6 weeks, or a few months at most.