openjournals / joss-reviews

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

[REVIEW]: Pyra: Automated EM27/SUN Greenhouse Gas Measurement Software #5131

Closed editorialbot closed 1 year ago

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@patrickjaigner<!--end-author-handle-- (Patrick Aigner) Repository: https://github.com/tum-esm/pyra Branch with paper.md (empty if default branch): joss-submission Version: 4.0.7 Editor: !--editor-->@AoifeHughes<!--end-editor-- Reviewers: @nmstreethran, @arthur-e Archive: 10.5281/zenodo.7825610

Status

status

Status badge code:

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

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

@nmstreethran & @arthur-e, 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 @AoifeHughes 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 @arthur-e

πŸ“ Checklist for @nmstreethran

editorialbot commented 1 year 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 1 year ago
Software report:

github.com/AlDanial/cloc v 1.88  T=0.14 s (1327.8 files/s, 97152.6 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
TypeScript                      69            263             55           3908
Python                          55            981            876           3681
Markdown                        24            561              0           1731
TeX                              1             34              0            340
JSON                            12              0              0            331
JavaScript                       6             16             13            257
CSS                              6             56              7            246
YAML                             4             13             11            145
TOML                             3             11              5             82
SVG                              3              0              0             29
HTML                             1              0              0             12
Rust                             2              1              0             12
Bourne Shell                     1              1              0              4
-------------------------------------------------------------------------------
SUM:                           187           1937            967          10778
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 1 year ago

Wordcount for paper.md is 1787

editorialbot commented 1 year ago

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

AoifeHughes commented 1 year ago

Hello everyone,

Thank you, @nmstreethran, @arthur-e for agreeing to review, we really appreciate your time and effort on this. Please ping me or let me know if there's anything I can do to help or any questions you may have.

Similarly, @patrickjaigner do let me know if you've anything you're unsure of, and I'll also be happy to help too πŸ˜„

AoifeHughes commented 1 year ago

Hello, I just wanted to check in @nmstreethran, @arthur-e is everything going okay or can I help at all?

arthur-e commented 1 year ago

@AoifeHughes Apologies for the delay. I hope to start the review this weekend.

arthur-e commented 1 year ago

Review checklist for @arthur-e

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper


Detailed Feedback

Regarding the Software Paper:

Overall, I think the authors could better describe the current practice for managing these CO2 monitoring stations. We are told that they "require at least one operator to be on-site for each instrument" but it's not clear how a single software library (Pyra) closes the gap between on-site operation and remote automation. Is there already some kind of common device firmware that can be accessed over Bluetooth or WiFi? I'm assuming Pyra isn't installed directly onto these devices, so what kind of middleware is running?

arthur-e commented 1 year ago

@patrickjaigner I've hit a blocking point in testing the Functionality of Pyra; please see this GitHub issue.

patrickjaigner commented 1 year ago

Thank you for your feedback @arthur-e .

We have already updated your points on the software paper. (Line 8-9, Line 28, Line 40, Line 41, Line 72, Line 99).

To give a better understanding of the overall system setup we created an overview and added it to the website (https://pyra.esm.ei.tum.de/docs/user-guide/functionality). The usual setup involves a Windows-based machine, an Ethernet + USB connection, and the EM27/SUN. In addition, a weather-protected enclosure needs to be present to be able to leave the instrument deployed at the measurement location. This weather-protected enclosure can be the TUM enclosure or any other custom-made enclosure by the community.

Measurements with an EM27/SUN require to run CamTracker, a software that tracks the sun position and operates the mirrors accordingly, and OPUS, a software that sends commands to the EM27/SUN and collects the output data into interferogram files. Both CamTracker and OPUS are Windows based and are used to operate the instrument.

Pyra allows to start and stop measurements with a single command through interaction with CamTracker and OPUS and monitors that both softwares act in the way they are supposed to. A common error for CamTracker is, for example, the loss of its tracking target (sun), which leads to no sunlight available and measurements failing for the rest of the day. In the named example, Pyra would detect that the tracker is not fixed on the sun anymore and restarts the CamTracker tracking.

Without Pyra operators would need to remotely connect to the windows machine at the start of the day (if a remote connection is available for the measurement location), initialize CamTracker, load a macro into OPUS, and start measurements. During the day, the operator would need to check the system to ensure that measurements are running and that no measurement-blocking issues are present.

Lines 53-61: Sun Elevation and Time can be set as the operator sees fit (domain knowledge). In our experience, a sun angle under 10Β° produces measurement files that are unusable in the post-processing. Sun Elevations above 10Β° are no problem. If multiple triggers are selected i.e. Time and Sun Elevation an AND connection is considered and both conditions need to be fulfilled for a measurement to start. We offer both Time and Sun Elevation for the operator to customize the measurement period if no auxiliary sensor information about the sun or cloud coverage is present. (https://pyra.esm.ei.tum.de/docs/user-guide/measurements/#measurement-modes-and-triggers)

Addition info: I will be on my annual vacation from 03.03 until 24.03. During this time my co-authors will be available to answer questions and react to blocking issues.

arthur-e commented 1 year ago

@patrickjaigner Thanks for the clarification.

Regarding the "Documentation" checklist above...

@AoifeHughes After the API documentation is found, I've taken my review as far as I can. I don't have access to Windows (or OPUS, or CamTracker) so I can't verify any of the items under "Functionality."

patrickjaigner commented 1 year ago

@arthur-e We didn't produce an explicit API documentation yet as we hoped that the source code and the CLI documentation would already act as comprehensive API documentation. We can create one if needed.

nmstreethran commented 1 year ago

Review checklist for @nmstreethran

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

AoifeHughes commented 1 year ago

Hello again, I just wanted to check in and see how the review is going @nmstreethran, @arthur-e.

nmstreethran commented 1 year ago

Apologies for the delay @AoifeHughes, I'll complete my review by the end of the week.

AoifeHughes commented 1 year ago

No worries :) thanks for letting me know

arthur-e commented 1 year ago

Hi @AoifeHughes I tagged you earlier--After the API documentation is found, I've taken my review as far as I can. I don't have access to Windows (or OPUS, or CamTracker) so I can't verify any of the items under "Functionality."

patrickjaigner commented 1 year ago

Dear reviewers, I have a short update from our side.

@nmstreethran Thank you for the detailed review and feedback. We are working on integrating your points.

@arthur-e We are currently working on adding an API documentation to the website. This should be ready by the end of the week. The draft for the pull request can be found here: https://github.com/tum-esm/pyra/pull/160

Regarding the functionality we can offer different approaches:

arthur-e commented 1 year ago

@AoifeHughes @patrickjaigner I'm not worried about the Functionality claims. Perhaps @nmstreethran can verify those claims and that is sufficient?

nmstreethran commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

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

AoifeHughes commented 1 year ago

Hi,

@nmstreethran , @arthur-e , given the PR that @patrickjaigner has mentioned (and has been merged) is this sufficient to help progress the review. If you still have concerns or questions, could you let me know.

@patrickjaigner please do let me (and the reviewers) know if you've any thoughts on the progress made.

patrickjaigner commented 1 year ago

hi @AoifeHughes, thank you for checking in. From our side, everything looks good. As far as I can tell from the checklist most points are checked already. We fixed an issue raised by @nmstreethran for the pyra-setup-tool. Also, the API documentation requested by @arthur-e is integrated into our website.

arthur-e commented 1 year ago

@AoifeHughes The changes to the API documentation look good. I think the paper and the repository should be accepted.

nmstreethran commented 1 year ago

Hi @AoifeHughes I've completed my review and would like to recommend that this submission be accepted. Many thanks.

AoifeHughes commented 1 year ago

@patrickjaigner this all looks great to me, whilst I'm doing some final checks could you make sure that:

patrickjaigner commented 1 year ago

Thank you for the update @AoifeHughes The latest tagged release is 4.0.7 which we published with Zenodo: 10.5281/zenodo.7825610 Link: https://doi.org/10.5281/zenodo.7825610

Let us know if we can help you with anything else.

AoifeHughes commented 1 year ago

@editorialbot set https://doi.org/10.5281/zenodo.7825610 as archive

editorialbot commented 1 year ago

Done! Archive is now https://doi.org/10.5281/zenodo.7825610

AoifeHughes commented 1 year ago

@editorialbot set 4.0.7 as version

editorialbot commented 1 year ago

Done! version is now 4.0.7

AoifeHughes commented 1 year ago

@editorialbot recommend-accept

editorialbot commented 1 year ago
Attempting dry run of processing paper acceptance...
editorialbot commented 1 year ago

:wave: @openjournals/ese-eics, this paper is ready to be accepted and published.

Check final proof :point_right::page_facing_up: Download article

If the paper PDF and the deposit XML files look good in https://github.com/openjournals/joss-papers/pull/4149, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

kthyng commented 1 year ago

Not sure if this matters but just in case...

kthyng commented 1 year ago

@editorialbot set 10.5281/zenodo.7825610 as archive

editorialbot commented 1 year ago

Done! Archive is now 10.5281/zenodo.7825610

kthyng commented 1 year ago

@editorialbot check references

kthyng commented 1 year ago

@editorialbot check references

kthyng commented 1 year ago

Hm I am not sure what is going on with the bot. @AoifeHughes were you able to check the references previously?

patrickjaigner commented 1 year ago

Is there anything that we can assist with from our side?

kthyng commented 1 year ago

Sorry for my delay, I had a stomach bug come through my household :(

kthyng commented 1 year ago

@editorialbot check references

kthyng commented 1 year ago

Hi @patrickjaigner! Not sure why the reference checker won't work. But can you update the metadata in your Zenodo archive so the title and author list exactly match your JOSS paper?

kthyng commented 1 year ago
patrickjaigner commented 1 year ago

Hi @kthyng, I updated the title and double-checked the author list.

kthyng commented 1 year ago

Ok we're ready for final publication!

kthyng commented 1 year ago

@editorialbot accept

editorialbot commented 1 year ago
Doing it live! Attempting automated processing of paper acceptance...
editorialbot commented 1 year ago

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: Aigner given-names: Patrick orcid: "https://orcid.org/0000-0002-1530-415X" - family-names: Makowski given-names: Moritz orcid: "https://orcid.org/0000-0002-2948-2993" - family-names: Luther given-names: Andreas orcid: "https://orcid.org/0000-0001-6382-0899" - family-names: Dietrich given-names: Florian orcid: "https://orcid.org/0000-0002-3069-9946" - family-names: Chen given-names: Jia orcid: "https://orcid.org/0000-0002-6350-6610" contact: - family-names: Aigner given-names: Patrick orcid: "https://orcid.org/0000-0002-1530-415X" - family-names: Chen given-names: Jia orcid: "https://orcid.org/0000-0002-6350-6610" doi: 10.5281/zenodo.7825610 message: If you use this software, please cite our article in the Journal of Open Source Software. preferred-citation: authors: - family-names: Aigner given-names: Patrick orcid: "https://orcid.org/0000-0002-1530-415X" - family-names: Makowski given-names: Moritz orcid: "https://orcid.org/0000-0002-2948-2993" - family-names: Luther given-names: Andreas orcid: "https://orcid.org/0000-0001-6382-0899" - family-names: Dietrich given-names: Florian orcid: "https://orcid.org/0000-0002-3069-9946" - family-names: Chen given-names: Jia orcid: "https://orcid.org/0000-0002-6350-6610" date-published: 2023-04-28 doi: 10.21105/joss.05131 issn: 2475-9066 issue: 84 journal: Journal of Open Source Software publisher: name: Open Journals start: 5131 title: "Pyra: Automated EM27/SUN Greenhouse Gas Measurement Software" type: article url: "https://joss.theoj.org/papers/10.21105/joss.05131" volume: 8 title: "Pyra: Automated EM27/SUN Greenhouse Gas Measurement Software" ```

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.