search

openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
714 stars 38 forks source link

[REVIEW]: pyGCodeDecode: A Python package for time-accurate GCode simulation in material extrusion processes #6465

Closed editorialbot closed 2 months ago

editorialbot commented 6 months ago

Submitting author: !--author-handle-->@FelixFroelich<!--end-author-handle-- (Felix Frölich) Repository: https://github.com/FAST-LB/pyGCodeDecode Branch with paper.md (empty if default branch): main Version: 0.2.4 Editor: !--editor-->@zhubonan<!--end-editor-- Reviewers: @Extraweich, @RahulSundar Archive: 10.5281/zenodo.12685261

Status

status

Status badge code:

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

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

@Extraweich & @RahulSundar, 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 @zhubonan 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 @Extraweich

📝 Checklist for @RahulSundar

editorialbot commented 6 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 6 months ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1007/978-3-030-56127-7 is OK
- 10.1007/s11665-023-08476-2 is OK
- 10.1108/RPJ-12-2018-0316 is OK

MISSING DOIs

- No DOI given, and none found for title: GRBL Firmware
- No DOI given, and none found for title: Marlin Firmware
- No DOI given, and none found for title: Marlin Documentation
- No DOI given, and none found for title: Klipper Documentation

INVALID DOIs

- doi.org/10.3139/120.111178 is INVALID because of 'doi.org/' prefix
editorialbot commented 6 months ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.04 s (885.8 files/s, 214134.8 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
SVG                              5              4             21           3176
Python                          16            541            766           1819
Markdown                         3            822              0           1268
YAML                            10             37             43            282
TOML                             1             13              1             67
TeX                              1              8              0             62
JSON                             1              0              0             14
-------------------------------------------------------------------------------
SUM:                            37           1425            831           6688
-------------------------------------------------------------------------------

Commit count by author:

   212  usmfi
   152  Lukas Hof
    14  lukashof
     9  felix.froelich
     7  Felix Frölich
     6  mn3560
     3  lukas.hof
     2  Jonathan Emil Knirsch
editorialbot commented 6 months ago

Paper file info:

📄 Wordcount for paper.md is 1405

🔴 Failed to discover a Statement of need section in paper

editorialbot commented 6 months ago

License info:

✅ License found: MIT License (Valid open source OSI approved license)

editorialbot commented 6 months ago

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

zhubonan commented 6 months ago

Hi @Extraweich, @RahulSundar Thanks again for agreeing to review this submission 👍. Please generate your checklists and get started!

It would be great to have the review done in two weeks time if possible. Please let me know if you have any problem or require a bit more time. Cheers 🚀 .

Extraweich commented 6 months ago

@editorialbot generate pdf

editorialbot commented 6 months ago

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

Extraweich commented 6 months ago

Review checklist for @Extraweich

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Extraweich commented 6 months ago

Hi @Extraweich, @RahulSundar Thanks again for agreeing to review this submission 👍. Please generate your checklists and get started!

It would be great to have the review done in two weeks time if possible. Please let me know if you have any problem or require a bit more time. Cheers 🚀 .

Hi there. Two weeks will be unlikely, because I am a bit busy currently with attending conferences. I think end of March would be possible :)

RahulSundar commented 6 months ago

Review checklist for @RahulSundar

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Extraweich commented 6 months ago

Hi everybody, Sorry for being absent for a while. I was busy with conferences and my own publications, but I will take my time in the following week to hopefully finish the review.

As a starting point I would like to highlight some issues in the paper:

In general, I am missing a comparison with other softwares which are out there and highlighting in what sense pyGCodeDecode is different (state of the field). Please add a section "Statement of Need" for this, which is required by JOSS.

Concerning the check point "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", it would be good to add a few more words in your README.md to clarify how exactly you expect people from the community to contribute to the project, or you could add a CONTRIBUTING.md (e.g. the one in my own repository as a reference).

Best regards Nicolas

FelixFroelich commented 6 months ago

Hi everybody, Sorry for being absent for a while. I was busy with conferences and my own publications, but I will take my time in the following week to hopefully finish the review.

As a starting point I would like to highlight some issues in the paper:

  • Line 49: it would be preferable to add some references or links for the given documentations/algorithms (i.e. MARLIN classic jerk, MARLIN junction deviation and Klipper). Not being part of the 3D printing research community, I would want something to read into.
  • Figure 2 and following equations: the figure is inconsistent in the sense that you are using tacc, tconst and tdec as time increments or intervals, but in the current notation they can easily be interpreted as specific locations in time. As such, it might add clearity if you would indicate their incremental nature, e.g. Δtacc, Δtconst and Δtdec, as it is also indicated in the figure itself with a=ΔvΔt. Also the font differs between figure and equations, since you are using italic subscripts in the figure, but not in the equations.
  • Line 67: what is indicted by "..." following GRBL/MARLIN?

In general, I am missing a comparison with other softwares which are out there and highlighting in what sense pyGCodeDecode is different (state of the field). Please add a section "Statement of Need" for this, which is required by JOSS.

Concerning the check point "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", it would be good to add a few more words in your README.md to clarify how exactly you expect people from the community to contribute to the project, or you could add a CONTRIBUTING.md (e.g. the one in my own repository as a reference).

Best regards Nicolas

Hi Nicolas,

Thank you very much for reviewing our paper.

We agree with all of your points and will incorporate them into the paper. As soon as the revised paper is in the main branch, I will let you know.

We will also create a CONTRIBUTING.md file based on your repository and add the missing information.

Best regards Felix

FelixFroelich commented 5 months ago

Hi everybody, Sorry for being absent for a while. I was busy with conferences and my own publications, but I will take my time in the following week to hopefully finish the review. As a starting point I would like to highlight some issues in the paper:

  • Line 49: it would be preferable to add some references or links for the given documentations/algorithms (i.e. MARLIN classic jerk, MARLIN junction deviation and Klipper). Not being part of the 3D printing research community, I would want something to read into.
  • Figure 2 and following equations: the figure is inconsistent in the sense that you are using tacc, tconst and tdec as time increments or intervals, but in the current notation they can easily be interpreted as specific locations in time. As such, it might add clearity if you would indicate their incremental nature, e.g. Δtacc, Δtconst and Δtdec, as it is also indicated in the figure itself with a=ΔvΔt. Also the font differs between figure and equations, since you are using italic subscripts in the figure, but not in the equations.
  • Line 67: what is indicted by "..." following GRBL/MARLIN?

In general, I am missing a comparison with other softwares which are out there and highlighting in what sense pyGCodeDecode is different (state of the field). Please add a section "Statement of Need" for this, which is required by JOSS. Concerning the check point "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", it would be good to add a few more words in your README.md to clarify how exactly you expect people from the community to contribute to the project, or you could add a CONTRIBUTING.md (e.g. the one in my own repository as a reference). Best regards Nicolas

Hi Nicolas,

Thank you very much for reviewing our paper.

We agree with all of your points and will incorporate them into the paper. As soon as the revised paper is in the main branch, I will let you know.

We will also create a CONTRIBUTING.md file based on your repository and add the missing information.

Best regards Felix

Hi everyone,

We have now revised the paper according to @Extraweich comments. We also added a CONTRIBUTING.md file and a Code_of_Conduct.md file to the repository.

All revisions have already been merged into the main branch. We hope all the changes are clear and understandable. If not, please do not hesitate to contact us with any questions!

Thanks for your helpful comments!

Best regards Felix

FelixFroelich commented 5 months ago

@editorialbot generate pdf

editorialbot commented 5 months ago

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

Extraweich commented 5 months ago

@zhubonan I have finished reviewing the paper and code. All issues opened in the repository have been solved and all checkpoints are satisfied. As such, I think that the paper is suitable and ready to be published in JOSS.

zhubonan commented 5 months ago

Hi @RahulSundar, how is your review going? Please let us know if you need any assistance. Thanks!

zhubonan commented 5 months ago

Hi @RahulSundar, how is your review going? Please let us know if you need any assistance. Thanks!

zhubonan commented 4 months ago

Pinging @RahulSundar again

zhubonan commented 4 months ago

Pinging @RahulSundar again ...

RahulSundar commented 4 months ago

Dear @zhubonan Thanks to the authors and editors for your patience. I am in the process of completion. I will complete the review in a few days and update. Regards, Rahul

RahulSundar commented 4 months ago

@editorialbot generate pdf

editorialbot commented 4 months ago

:warning: An error happened when generating the pdf.

RahulSundar commented 4 months ago

Some preliminary comments are as follows:

  1. Please do add some key references in the main README.md file as well of your repository.
  2. Also, request that any set of step-by instructions are presented in the README.md as an enumerated list and utilize appropriate grammar, punctuation and capitalizations in the instructions where necessary.
  3. Although a statement of need is provided where it is mentioned the other software are inaccurate, it would be useful to see an image comparing them in the validation section. This would further strengthen the claim of the paper.
  4. In addition to showing a comparison, the errors need to be tabulated as well.
RahulSundar commented 4 months ago

⚠️ An error happened when generating the pdf.

@zhubonan why is this happening?

zhubonan commented 4 months ago

@editorialbot generate pdf

editorialbot commented 4 months ago

:warning: An error happened when generating the pdf.

lukashof commented 4 months ago

Dear @RahulSundar, thank you for your remarks! We will look at what we can do to address those as soon as possible and come back to you. We are looking forward to your further input! 🙂

Dear @zhubonan, we don't understand why the PDF-generation is failing. The error message states that the repo couldn't be downloaded, but we didn't change the URL. Was there some recent change to the bot?

zhubonan commented 4 months ago

Have you tried to generate the pdf (the simplest (the easiest way is to use docker)? If you can do that, feel free to attach it to this issue as a temporary workaround.

lukashof commented 4 months ago

Yes, we compile a draft with every commit to main in our workflow. I'm attaching the most recent version. pyGCD_paper.pdf

lukashof commented 4 months ago

I was just wondering whether we did anything wrong or the bot might be broken.

danielskatz commented 4 months ago

I think there's something broken, and we're aware of it and working on it.

lukashof commented 4 months ago

Thank you for letting us know, @danielskatz !

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

danielskatz commented 4 months ago

This seems to be working now, so perhaps it was a transient error.

Also, when one of us cloned the repo locally, we got this warning message:

Encountered 3 file(s) that should have been pointers, but weren't:
    tests/data/test_gcode_interpreter.gcode
    tests/data/test_state_generator.gcode
    tests/self_test/self_test.gcode
lukashof commented 4 months ago

Great 👍!

Thanks for making us aware of the warning! Some files were older than the git LFS configuration, and therefore not yet tracked as intended. The warning should be gone now.

RahulSundar commented 4 months ago

Thanks @danielskatz !

On a further look, I don't have anymore comments to add. Only that a brief quantitative/qualitative comparison with existing software is missing. Request the authors to make corrections based on my previous suggestions. I have put some items on my checklist on hold until that is completed.

zhubonan commented 4 months ago

Thanks @RahulSundar for the review! @lukashof Please let me know when you have finished updating 🚀 .

lukashof commented 3 months ago

Some preliminary comments are as follows:

1. Please do add some key references in the main README.md file as well of your repository.

2. Also, request that any set of step-by instructions are presented in the README.md as an enumerated list and utilize appropriate grammar, punctuation and capitalizations in the instructions where necessary.

3. Although a statement of need is provided where it is mentioned the other software are inaccurate, it would be useful to see an image comparing them in the validation section. This would further strengthen the claim of the paper.

4. In addition to showing a comparison, the errors need to be tabulated as well.

Hello @RahulSundar and @zhubonan,

it took us a bit to get this done, but we finally addressed the comments.

Regarding 1. and 2.: We reworked the README. It should now introduce the package in a more concise way and be more readable overall.

Regarding 3.: The image comparing the state-of-the-art approach of simply displaying the target velocities from the GCode with our simulation is now also referenced in the validation section. It would be possible to move the image to that section entirely to distinguish the sections more clearly, but we felt, that it also adds value to the statement of need. We hope you agree.

Regarding 4.: The maximum error between our simulation and the experiment shown in the chart is now part of the text. We are already exceeding the length limit set by FOSS and therefore didn't add a table. The errors will also depend on the exact GCode and printer settings chosen for the test case. For that reason, we would like to avoid putting too much emphasis on the exact deviation from the experiment.

Thanks again for your invaluable feedback, @RahulSundar! We hope our changes meet your expectations. Let us know if you feel we missed something!

lukashof commented 3 months ago

@zhubonan,

while working on the changes mentioned above, we realized that the page number in the footer seems to behave oddly in our compiled paper.

grafik

However, it seems fine in the version compiled by the bot in this thread. Do we have something misconfigured, or is this expected behavior?

zhubonan commented 3 months ago

@lukashof Thanks for the update!

while working on the changes mentioned above, we realized that the page number in the footer seems to behave oddly in our compiled paper.

I think it is just a placeholder for the page number. Your paper has not been published, hence it does not have a page number. The PDF generated by the bot does have a page number (a fake one, probably) though. Don't worry, we will run a set of final checks after acceptance.

RahulSundar commented 3 months ago

Dear All, I have completed the review. The authors have answered my queries suitably. I find the paper suitable to be published in JOSS.

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

FelixFroelich commented 3 months ago

Thank you very much for the review! We are pleased that the paper can be published!

zhubonan commented 3 months ago

Just to give you a heads-up - I will run some additional checks then we can move to the next stage!

zhubonan commented 3 months ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance