Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[REVIEW]: cppTPSA/pyTPSA: a C++/Python package for truncated power series algebra #4818

Closed
editorialbot opened this issue Oct 5, 2022 · 172 comments
Closed

[REVIEW]: cppTPSA/pyTPSA: a C++/Python package for truncated power series algebra #4818

editorialbot opened this issue Oct 5, 2022 · 172 comments
Assignees
@danielskatz @mbkumar @CFGrote
Labels
accepted C++ CMake published Papers published in JOSS recommend-accept Papers recommended for acceptance in JOSS. review Shell Track: 7 (CSISM) Computer science, Information Science, and Mathematics

Comments

@editorialbot
Copy link
Collaborator

editorialbot commented Oct 5, 2022 •
edited

Submitting author: @zhanghe9704 (He Zhang)
Repository: https://github.com/zhanghe9704/tpsa
Branch with paper.md (empty if default branch):
Version: v1.1.1
Editor: @danielskatz
Reviewers: @CFGrote, @mbkumar
Archive: 10.5281/zenodo.10728770

Status

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: [![status](https://joss.theoj.org/papers/1fa756415c650531cdfd292233b4cc8d/status.svg)](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

📝 Checklist for @CFGrote

📝 Checklist for @mbkumar
@editorialbot editorialbot added C++ CMake review Shell Track: 7 (CSISM) Computer science, Information Science, and Mathematics labels Oct 5, 2022
@editorialbot editorialbot mentioned this issue Oct 5, 2022
Closed
@editorialbot
Copy link
Collaborator Author

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
Copy link
Collaborator Author 

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

@editorialbot
Copy link
Collaborator Author

Wordcount for paper.md is 1523

@editorialbot
Copy link
Collaborator Author 

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

@editorialbot
Copy link
Collaborator Author

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@pibion
Copy link

pibion commented Oct 6, 2022

@chrisrogers1234, could you try generating your review checklist and confirm that you can post on this thread?

@pibion
Copy link

pibion commented Oct 6, 2022

@dtabell, are you still able to review? I know it's been a long time since you agreed to review.

@chrisrogers1234
Copy link

@pibion I can post. However, I am full this week and next and unable to get started on this until w/c 17th October.

@dtabell
Copy link

dtabell commented Oct 7, 2022 via email

@pibion
Copy link

pibion commented Oct 10, 2022

@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!

@chrisrogers1234
Copy link

@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!

@chrisrogers1234
Copy link

@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

@zhanghe9704
Copy link

zhanghe9704 commented Oct 18, 2022 via email

@chrisrogers1234
Copy link

@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.

@pibion
Copy link

pibion commented Oct 27, 2022

@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.

@pibion
Copy link

pibion commented Oct 31, 2022

@zhanghe9704 floating the request to add the makefile to your repo up to the top. Thank you!

@zhanghe9704
Copy link

zhanghe9704 commented Nov 1, 2022 via email

@pibion
Copy link

pibion commented Nov 3, 2022

@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?

@pibion
Copy link

pibion commented Nov 3, 2022

@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!

@zhanghe9704
Copy link

zhanghe9704 commented Nov 3, 2022 via email

@danielskatz
Copy link

Hi @pibion - what's the status of this? I don't see any reviewer checklists which confuses me.

@danielskatz
Copy link

👋 @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?

@chrisrogers1234
Copy link

@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

@CFGrote
Copy link

CFGrote commented Feb 28, 2024

Checked all boxes, I recommend publish the paper as is. Thanks!

@danielskatz
Copy link

Thanks @mbkumar and @CFGrote for your work in your reviews

@danielskatz
Copy link

@editorialbot generate pdf

@danielskatz
Copy link

@editorialbot check references

@editorialbot
Copy link
Collaborator Author

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@editorialbot
Copy link
Collaborator Author 

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
- 10.1109/PAC.2003.1289960 is OK
- 10.5281/zenodo.7900975 is OK

MISSING DOIs

- None

INVALID DOIs

- None

@danielskatz
Copy link

@zhanghe9704 - I've proofread the paper, and have suggested changes in zhanghe9704/tpsa#17 - please merge this, or let me know what you disagree with, then we can proceed to the final steps.

@zhanghe9704
Copy link

zhanghe9704 commented Feb 28, 2024 via email

@danielskatz
Copy link

@zhanghe9704 - At this point could you:

  • Make a tagged release of your software, and list the version tag of the archived version here.
  • Archive the reviewed software in Zenodo or a similar service (e.g., figshare, an institutional repository)
  • Check the archival deposit (e.g., in Zenodo) has the correct metadata. This includes the title (should match the paper title) and author list (make sure the list is correct and people who only made a small fix are not on it). You may also add the authors' ORCID.
  • Please list the DOI of the archived version here.

I can then move forward with accepting and publishing the submission.

@zhanghe9704
Copy link

Version tag: v1.1.1
DOI: 10.5281/zenodo.10728770

Please let me know if anything is wrong or missing. Thanks a lot!

@danielskatz
Copy link

@editorialbot set v1.1.1 as version

@editorialbot
Copy link
Collaborator Author

Done! version is now v1.1.1

@danielskatz
Copy link

@editorialbot set 10.5281/zenodo.10728770 as archive

@editorialbot
Copy link
Collaborator Author

Done! archive is now 10.5281/zenodo.10728770

@danielskatz
Copy link

@editorialbot recommend-accept

@editorialbot
Copy link
Collaborator Author 

Attempting dry run of processing paper acceptance...

@editorialbot
Copy link
Collaborator Author 

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
- 10.1109/PAC.2003.1289960 is OK
- 10.5281/zenodo.7900975 is OK

MISSING DOIs

- None

INVALID DOIs

- None

@editorialbot
Copy link
Collaborator Author

👋 @openjournals/csism-eics, this paper is ready to be accepted and published.

Check final proof 👉📄 Download article

If the paper PDF and the deposit XML files look good in openjournals/joss-papers#5075, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

@editorialbot editorialbot added the recommend-accept Papers recommended for acceptance in JOSS. label Feb 29, 2024
@danielskatz
Copy link

@editorialbot accept

@editorialbot
Copy link
Collaborator Author 

Doing it live! Attempting automated processing of paper acceptance...

@editorialbot
Copy link
Collaborator Author

Ensure proper citation by uploading a plain text CITATION.cff file to the default branch of your repository.

If using GitHub, a Cite this repository menu will appear in the About section, containing both APA and BibTeX formats. When exported to Zotero using a browser plugin, Zotero will automatically create an entry using the information contained in the .cff file.

You can copy the contents for your CITATION.cff file here:

CITATION.cff 

cff-version: "1.2.0"
authors:
- family-names: Zhang
  given-names: He
  orcid: "https://orcid.org/0000-0001-7701-4118"
contact:
- family-names: Zhang
  given-names: He
  orcid: "https://orcid.org/0000-0001-7701-4118"
doi: 10.5281/zenodo.10728770
message: If you use this software, please cite our article in the
  Journal of Open Source Software.
preferred-citation:
  authors:
  - family-names: Zhang
    given-names: He
    orcid: "https://orcid.org/0000-0001-7701-4118"
  date-published: 2024-02-29
  doi: 10.21105/joss.04818
  issn: 2475-9066
  issue: 94
  journal: Journal of Open Source Software
  publisher:
    name: Open Journals
  start: 4818
  title: "cppTPSA/pyTPSA: a C++/Python package for truncated power
    series algebra"
  type: article
  url: "https://joss.theoj.org/papers/10.21105/joss.04818"
  volume: 9
title: "cppTPSA/pyTPSA: a C++/Python package for truncated power series
  algebra"

If the repository is not hosted on GitHub, a .cff file can still be uploaded to set your preferred citation. Users will be able to manually copy and paste the citation.

Find more information on .cff files here and here.

@editorialbot
Copy link
Collaborator Author

🐘🐘🐘 👉 Toot for this paper 👈 🐘🐘🐘

@editorialbot
Copy link
Collaborator Author

🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨

Here's what you must now do:

  1. Check final PDF and Crossref metadata that was deposited 👉 Creating pull request for 10.21105.joss.04818 joss-papers#5076
  2. Wait five minutes, then verify that the paper DOI resolves https://doi.org/10.21105/joss.04818
  3. If everything looks good, then close this review issue.
  4. Party like you just published a paper! 🎉🌈🦄💃👻🤘

Any issues? Notify your editorial technical team...

@editorialbot editorialbot added accepted published Papers published in JOSS labels Feb 29, 2024
@danielskatz
Copy link

Congratulations to @zhanghe9704 (He Zhang) on your publication!!

And thanks to @CFGrote and @mbkumar for reviewing!
We depend on volunteers and couldn't do this without you

@editorialbot
Copy link
Collaborator Author

🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](https://joss.theoj.org/papers/10.21105/joss.04818/status.svg)](https://doi.org/10.21105/joss.04818)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.04818">
  <img src="https://joss.theoj.org/papers/10.21105/joss.04818/status.svg" alt="DOI badge" >
</a>

reStructuredText:
.. image:: https://joss.theoj.org/papers/10.21105/joss.04818/status.svg
   :target: https://doi.org/10.21105/joss.04818

This is how it will look in your documentation:

DOI

We need your help!

The 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:

@zhanghe9704
Copy link

zhanghe9704 commented Feb 29, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@danielskatz danielskatz

@mbkumar mbkumar

@CFGrote CFGrote

Labels
accepted C++ CMake published Papers published in JOSS recommend-accept Papers recommended for acceptance in JOSS. review Shell Track: 7 (CSISM) Computer science, Information Science, and Mathematics
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests