Closed editorialbot closed 2 years 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
Software report:
github.com/AlDanial/cloc v 1.88 T=0.07 s (273.2 files/s, 26939.2 lines/s)
-------------------------------------------------------------------------------
Language files blank comment code
-------------------------------------------------------------------------------
Python 15 218 294 967
Markdown 2 59 0 277
TeX 1 7 0 81
Bourne Shell 1 3 0 43
YAML 1 1 4 18
-------------------------------------------------------------------------------
SUM: 20 288 298 1386
-------------------------------------------------------------------------------
gitinspector failed to run statistical information for the repository
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):
OK DOIs
- 10.1016/S0003-2670(03)00676-7 is OK
- 10.1049/el:20062442 is OK
- 10.1145/1731022.1731030 is OK
- 10.1109/IMTC.2000.848895 is OK
MISSING DOIs
- None
INVALID DOIs
- 10.1016/j.applthermaleng.2004.03.001 is INVALID
- 10.1007/978-3-642-23099-8 is INVALID
Wordcount for paper.md
is 1206
:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
@gdetor and @thejackal360 would you rather all comments be made here or in issues on the repository?
The paper is generally well written, but there are issues with the documentation that make the using the code difficult for a new user
1) The installation instructions presented in the README
only works if one has the right version of gcc (I think it has to be 8.2.1+ because of GLIBC_2.29). This is because instead of providing the source code for the modified version of ngspice
as a submodule or an external code, the repository includes libngspice.so
which was compiled with gcc-9.4. This is a major limiting factor for installing the code, and I think that the ngspice library's source code should be provided, including instructions on how to compile it. Ideally this would be done automatically with pip
, but that would be non-trivial to set up.
2) The README has a few typos in it, including links to the private repository for OpenPelt.
3) While the tests are automated, they are done with a seperate bash script that overrides LD_LIBRARY_PATH
to include the provided libngspice.so
file. If you have to compile ngspice from source this breaks the script. I'd recommend converting the existing testing protocol to use pytest
4) The provided examples and API are not enough for a newcomer to understand how to use the code. I'd include a description of the physical model that is being modeled in the examples. Improvements to the python API would also be helpful as it is currently incomplete.
I opened issues on the repository for these problems
Thank you for your feedback! We've resolved all the issues on the OpenPelt repo.
There are still a few functions in controller.py
and all of neural_network.py
that are undocumented
@tpurcell90 Thank you for pointing out that issue. We fixed that.
@tpurcell90, many thanks for starting your review so promptly! @danaraujocr, please don't hesitate to ask if you need any clarifications on how the process works.
:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
All my comments have been addressed.
π Hi @danaraujocr, just checking in on how your review is going!
Hi Rachel,
Sorry about the delay. Had an issue with my computer and just solved it. Will continue the work later today.
@gdetor and @thejackal360 The paper is well written, but the installation doc are still not perfectly clear. I ran a new ubuntu 20.04 installation on a virtual machine to check if the installation would run exactly as described but it didn't for minor reasons. The few things I would suggest adding/changing are: 1 - in the build_nspice.sh file a YACC command is called and those were not available in the build I used. Easily solvable. I commented on the issue opened by @tpurcell90 adding this complication. 2 - In the instructions to add the lbngspice environment variable the text suggests that one needs to copy/paste the snippet into .bashrc and what is in it is a command that does that for you. that might be confusing for some users. 3 - In the snippet that contains the code below : $ cd OpenPelt/ $ ./build_ngspice.sh $ pip3 (or pip) install . I think you should only keep the last line, as the two previous lines have probably already been executed. (especially the shell script)
Other than those minor things, nothing else to add.
@danaraujocr Thank you for your comments/suggestions. We updated the README file including all your suggestions.
@rkurchin no other commentaries to make.
Thanks everyone! Authors, I'll do an editorial pass over the manuscript and send any comments shortly. In the meantime, the next steps for you are:
@editorialbot check references
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):
OK DOIs
- 10.1016/j.applthermaleng.2004.03.001 is OK
- 10.1016/S0003-2670(03)00676-7 is OK
- 10.1049/el:20062442 is OK
- 10.1007/978-3-642-23099-8 is OK
- 10.1145/1731022.1731030 is OK
- 10.1109/IMTC.2000.848895 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:
Some small editorial comments:
[@doe99; @smith2000; @smith2004]
) to get them all in one set of parentheses)` to get monospace typesetting, e.g. for mentions of
tec_plant` et al.@rkurchin Thank you for the suggestions/comments. We have incorporated all your suggestions to the text. Soon we will provide the DOI and the tag.
@editorialbot set beta as version
I'm sorry @thejackal360, I'm afraid I can't do that. That's something only editors are allowed to do.
Version Number: beta DOI: 10.5281/zenodo.6514903
Are we able to change the version to "beta" for the submission? JOSS won't let us use "@editorialbot set beta as version" since we are not editors.
@editorialbot set beta as version
Done! version is now beta
@editorialbot set 10.5281/zenodo.6514903 as archive
Done! Archive is now 10.5281/zenodo.6514903
@editorialbot generate pdf
:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
@editorialbot recommend-accept
Attempting dry run of processing paper acceptance...
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):
OK DOIs
- 10.1016/j.applthermaleng.2004.03.001 is OK
- 10.1016/S0003-2670(03)00676-7 is OK
- 10.1049/el:20062442 is OK
- 10.1007/978-3-642-23099-8 is OK
- 10.1145/1731022.1731030 is OK
- 10.1109/IMTC.2000.848895 is OK
MISSING DOIs
- None
INVALID DOIs
- None
:wave: @openjournals/joss-eics, this paper is ready to be accepted and published.
Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/3191
If the paper PDF and the deposit XML files look good in https://github.com/openjournals/joss-papers/pull/3191, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept
Hi @gdetor, I'm the AEIC on duty this week doing some final checks.
It looks like one reference is missing a DOI: Degrave et al. 2022. (Not sure why the automated check missed this.) Can you add this?
@kyleniemeyer Thank you for pointing that out. We added the missing DOI.
@editorialbot generate pdf
:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
@editorialbot accept
Doing it live! Attempting automated processing of paper acceptance...
π¦π¦π¦ π Tweet for this paper π π¦π¦π¦
π¨π¨π¨ THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! π¨π¨π¨
Here's what you must now do:
Any issues? Notify your editorial technical team...
Congratulations @gdetor on your article's publication in JOSS!
Many thanks to @tpurcell90 and @danaraujocr for reviewing this, and @rkurchin for editing it.
:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:
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.04306/status.svg)](https://doi.org/10.21105/joss.04306)
HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.04306">
<img src="https://joss.theoj.org/papers/10.21105/joss.04306/status.svg" alt="DOI badge" >
</a>
reStructuredText:
.. image:: https://joss.theoj.org/papers/10.21105/joss.04306/status.svg
:target: https://doi.org/10.21105/joss.04306
This is how it will look in your documentation:
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:
Submitting author: !--author-handle-->@gdetor<!--end-author-handle-- (Georgios Detorakis) Repository: https://github.com/thejackal360/OpenPelt-Public Branch with paper.md (empty if default branch): master Version: beta Editor: !--editor-->@rkurchin<!--end-editor-- Reviewers: @tpurcell90, @danaraujocr Archive: 10.5281/zenodo.6514903
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
@tpurcell90 & @danaraujocr, 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 @rkurchin 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 @tpurcell90
π Checklist for @danaraujocr