search

openjournals / joss-reviews

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

[REVIEW]: DiscreteEntropy.jl: Entropy Estimation of Discrete Random Variables with Julia #7334

Closed editorialbot closed 1 week ago

editorialbot commented 1 month ago

Submitting author: !--author-handle-->@kellino<!--end-author-handle-- (David Kelly) Repository: https://github.com/kellino/DiscreteEntropy.jl Branch with paper.md (empty if default branch): paper Version: v0.2.1 Editor: !--editor-->@Nikoleta-v3<!--end-editor-- Reviewers: @niyiyu, @nluetts Archive: 10.5281/zenodo.14067392

Status

status

Status badge code:

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

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

@niyiyu & @nluetts, 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 @Nikoleta-v3 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 @nluetts

📝 Checklist for @niyiyu

editorialbot commented 1 month 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 month ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.03 s (1566.9 files/s, 142880.1 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Julia                           26            649            625           1576
TOML                             5            173              2            718
Markdown                        12            101              0            298
YAML                             5             10             22            176
TeX                              1             12              0            106
-------------------------------------------------------------------------------
SUM:                            49            945            649           2874
-------------------------------------------------------------------------------

Commit count by author:

   215  David Kelly
    24  IlariaLaTorre
    23  =
     5  CompatHelper Julia
editorialbot commented 1 month ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

✅ OK DOIs

- 10.1137/1104033 is OK
- 10.1145/3368089.3409748 is OK
- 10.1109/tit.2004.834752 is OK
- 10.3390/e23050561 is OK
- 10.1023/A:1026096204727 is OK
- 10.7551/mitpress/1120.003.0065 is OK
- 10.1162/NECO_a_00266 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: Hyperfuzzing: black-box security hypertesting with...
- No DOI given, and none found for title: Entropy Estimates from Insufficient Samplings
- No DOI given, and none found for title: Entropy inference and the James-Stein estimator, w...

❌ MISSING DOIs

- None

❌ INVALID DOIs

- None
editorialbot commented 1 month ago

Paper file info:

📄 Wordcount for paper.md is 634

✅ The paper includes a Statement of need section

editorialbot commented 1 month ago

License info:

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

Nikoleta-v3 commented 1 month ago

Hey @niyiyu, @nluetts (@kellino) this is the review thread for the paper. All of our communications will happen here from now on.

As a reviewer, the first step is to create a checklist for your review by entering

@editorialbot generate my checklist

as the top of a new comment in this thread.

These checklists contain the JOSS requirements ✅ As you go over the submission, please check any items that you feel have been satisfied. The first comment in this thread also contains 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, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention https://github.com/openjournals/joss-reviews/issues/7334 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 any 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 (@Nikoleta-v3) if you have any questions/concerns. 😄 🙋🏻

editorialbot commented 1 month ago

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

nluetts commented 1 month ago

Review checklist for @nluetts

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

niyiyu commented 1 month ago

Review checklist for @niyiyu

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

niyiyu commented 1 month ago

@kellino Thank you for submitting this Julia package. I only have several minor suggestions to meet the JOSS criteria.

  1. Equation (1) is not correctly rendered in the PDF
  2. NSB is not correctly referenced in line 14 @nemenman2002entropy
  3. Please provide guidelines on how to contribute and report issues, ideally in the README files.

Best, Yiyu

kellino commented 1 month ago

@niyiyu thank you for the helpful comments. This should all be fixed in the both the paper and main branch.

niyiyu commented 1 month ago

@editorialbot generate pdf

editorialbot commented 1 month ago

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

niyiyu commented 1 month ago

Thanks for the updates. The equation is still not rendering thought. Other than that I don't have any concerns and will recommend for accepting. @Nikoleta-v3

kellino commented 1 month ago

Thank you @niyiyu. The equation should be rendering correctly now, as of the most recent commit https://github.com/kellino/DiscreteEntropy.jl/commit/ed5577923c3b5659a881701c0deb1329fa86b0b2

nluetts commented 3 weeks ago

@editorialbot generate pdf

editorialbot commented 3 weeks ago

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

nluetts commented 3 weeks ago

@kellino fixed all issues that came up during my review, so I finished my checklist.

The package performs as described in the paper, it comes with comprehensive documentation, and should be a very helpful tool for people working with entropy estimation. @Nikoleta-v3 I recommend the paper for acceptance.

kellino commented 2 weeks ago

@Nikoleta-v3 I just wanted to check what else is needed from me at this point, or should I just wait for a decision?

Nikoleta-v3 commented 2 weeks ago

👋🏻 @kellino

First of all, a huge thank you to the reviewers, @nluetts and @niyiyu, for their quick and detailed reviews! Your time and effort are very much appreciated.

I’d like to do one final read-through of the manuscript before we proceed 😄 Apologies for the delays on my part; I’m currently in the process of moving countries. However, I believe we can have the paper accepted by the end of the week.

Nikoleta-v3 commented 2 weeks ago

Well, this (https://github.com/openjournals/joss-reviews/issues/7334#issuecomment-2458929644) didn’t age well 😅 @kellino, I’ve opened a pull request and I just saw that you accepted the changes 👍🏻

In Markdown, you can set the code environment to Julia for better syntax highlighting 😄

At this point could you please:

kellino commented 2 weeks ago

@Nikoleta-v3 I think a small version bump to v0.2.1 is in order as there were many bug fixes. I've uploaded the software to https://doi.org/10.5281/zenodo.14067392 with v0.2.1 in the Project.toml metadata. Hopefully everything else should be correct!

Nikoleta-v3 commented 2 weeks ago

@kellino, on Zenodo (https://doi.org/10.5281/zenodo.14067392), you used the "Creative Commons Attribution" license, but your software on GitHub is under the MIT license. I believe JOSS requires them to be the same 😄

kellino commented 2 weeks ago

@kellino, on Zenodo (https://doi.org/10.5281/zenodo.14067392), you used the "Creative Commons Attribution" license, but your software on GitHub is under the MIT license. I believe JOSS requires them to be the same 😄

Done!

Nikoleta-v3 commented 1 week ago

@editorialbot set v0.2.1 as version

editorialbot commented 1 week ago

Done! version is now v0.2.1

Nikoleta-v3 commented 1 week ago

@editorialbot set 10.5281/zenodo.14067392 as archive

editorialbot commented 1 week ago

Done! archive is now 10.5281/zenodo.14067392

Nikoleta-v3 commented 1 week ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

Nikoleta-v3 commented 1 week ago

@editorialbot generate pdf

editorialbot commented 1 week ago

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

Nikoleta-v3 commented 1 week ago

@editorialbot check references

editorialbot commented 1 week ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

✅ OK DOIs

- 10.1137/1104033 is OK
- 10.1145/3368089.3409748 is OK
- 10.1109/tit.2004.834752 is OK
- 10.3390/e23050561 is OK
- 10.1023/A:1026096204727 is OK
- 10.7551/mitpress/1120.003.0065 is OK
- 10.1162/NECO_a_00266 is OK
- 10.18637/jss.v067.i08 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: Entropy Estimates from Insufficient Samplings
- No DOI given, and none found for title: Entropy inference and the James-Stein estimator, w...
- No DOI given, and none found for title: Elements of Information Theory 2nd Edition (Wiley ...
- No DOI given, and none found for title: Entropy inference and the James-Stein estimator, w...
- No DOI given, and none found for title: PYM entropy estimator MATLAB reference implementat...
- No DOI given, and none found for title: NSB Entropy Estimation
- No DOI given, and none found for title: Unseen
- No DOI given, and none found for title: ndd - Bayesian entropy estimation from discrete da...
- No DOI given, and none found for title: Estimating the Unseen: Improved Estimators for Ent...

❌ MISSING DOIs

- 10.1007/s10664-024-10556-3 may be a valid DOI for title: Hyperfuzzing: black-box security hypertesting with...
- 10.2307/j.ctv2rr3j2g.3 may be a valid DOI for title: BUB

❌ INVALID DOIs

- None
Nikoleta-v3 commented 1 week ago 
- 10.1007/s10664-024-10556-3 may be a valid DOI for title: Hyperfuzzing: black-box security hypertesting with...
- 10.2307/j.ctv2rr3j2g.3 may be a valid DOI for title: BUB

I have checked and the above suggestions are incorrect 😄

Nikoleta-v3 commented 1 week ago

@editorialbot recommend-accept

editorialbot commented 1 week ago 
Attempting dry run of processing paper acceptance...
editorialbot commented 1 week ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

✅ OK DOIs

- 10.1137/1104033 is OK
- 10.1145/3368089.3409748 is OK
- 10.1109/tit.2004.834752 is OK
- 10.3390/e23050561 is OK
- 10.1023/A:1026096204727 is OK
- 10.7551/mitpress/1120.003.0065 is OK
- 10.1162/NECO_a_00266 is OK
- 10.18637/jss.v067.i08 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: Entropy Estimates from Insufficient Samplings
- No DOI given, and none found for title: Entropy inference and the James-Stein estimator, w...
- No DOI given, and none found for title: Elements of Information Theory 2nd Edition (Wiley ...
- No DOI given, and none found for title: Entropy inference and the James-Stein estimator, w...
- No DOI given, and none found for title: PYM entropy estimator MATLAB reference implementat...
- No DOI given, and none found for title: NSB Entropy Estimation
- No DOI given, and none found for title: Unseen
- No DOI given, and none found for title: ndd - Bayesian entropy estimation from discrete da...
- No DOI given, and none found for title: Estimating the Unseen: Improved Estimators for Ent...

❌ MISSING DOIs

- 10.1007/s10664-024-10556-3 may be a valid DOI for title: Hyperfuzzing: black-box security hypertesting with...
- 10.2307/j.ctv2rr3j2g.3 may be a valid DOI for title: BUB

❌ INVALID DOIs

- None
editorialbot commented 1 week ago

:warning: Error preparing paper acceptance. The generated XML metadata file is invalid. 

ID ref-hausser2009entropy already defined
kellino commented 1 week ago

@Nikoleta-v3 the doi for Hyperfuzzing is correct (and new as of last week) so please give me a bit of time to put that into the paper draft.

Nikoleta-v3 commented 1 week ago

Thank you @kellino! For some reason when I search for the DOI this publication came up: https://dl.acm.org/doi/10.1145/3551349.3556947 but I can see now that it is the correct one

Nikoleta-v3 commented 1 week ago

@kellino in your bib file you also have the reference:

@misc{hausser2009entropy,
      title={Entropy inference and the {J}ames-{S}tein estimator, with application to nonlinear gene association networks},
      author={Jean Hausser and Korbinian Strimmer},
      year={2009},
      eprint={0811.3579},
      archivePrefix={arXiv},
      primaryClass={stat.ML}
}

twice. Could you also fix that please?

kellino commented 1 week ago

@Nikoleta-v3 both of those things should now be fixed.

Nikoleta-v3 commented 1 week ago

@editorialbot check references

editorialbot commented 1 week ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

✅ OK DOIs

- 10.1137/1104033 is OK
- 10.1145/3368089.3409748 is OK
- 10.1007/s10664-024-10556-3 is OK
- 10.1109/tit.2004.834752 is OK
- 10.3390/e23050561 is OK
- 10.1023/A:1026096204727 is OK
- 10.7551/mitpress/1120.003.0065 is OK
- 10.1162/NECO_a_00266 is OK
- 10.18637/jss.v067.i08 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: Entropy Estimates from Insufficient Samplings
- No DOI given, and none found for title: Elements of Information Theory 2nd Edition (Wiley ...
- No DOI given, and none found for title: Entropy inference and the James-Stein estimator, w...
- No DOI given, and none found for title: PYM entropy estimator MATLAB reference implementat...
- No DOI given, and none found for title: NSB Entropy Estimation
- No DOI given, and none found for title: Unseen
- No DOI given, and none found for title: ndd - Bayesian entropy estimation from discrete da...
- No DOI given, and none found for title: Estimating the Unseen: Improved Estimators for Ent...

❌ MISSING DOIs

- 10.2307/j.ctv2rr3j2g.3 may be a valid DOI for title: BUB

❌ INVALID DOIs

- None
Nikoleta-v3 commented 1 week ago

@editorialbot recommend-accept

editorialbot commented 1 week ago 
Attempting dry run of processing paper acceptance...
editorialbot commented 1 week ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

✅ OK DOIs

- 10.1137/1104033 is OK
- 10.1145/3368089.3409748 is OK
- 10.1007/s10664-024-10556-3 is OK
- 10.1109/tit.2004.834752 is OK
- 10.3390/e23050561 is OK
- 10.1023/A:1026096204727 is OK
- 10.7551/mitpress/1120.003.0065 is OK
- 10.1162/NECO_a_00266 is OK
- 10.18637/jss.v067.i08 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: Entropy Estimates from Insufficient Samplings
- No DOI given, and none found for title: Elements of Information Theory 2nd Edition (Wiley ...
- No DOI given, and none found for title: Entropy inference and the James-Stein estimator, w...
- No DOI given, and none found for title: PYM entropy estimator MATLAB reference implementat...
- No DOI given, and none found for title: NSB Entropy Estimation
- No DOI given, and none found for title: Unseen
- No DOI given, and none found for title: ndd - Bayesian entropy estimation from discrete da...
- No DOI given, and none found for title: Estimating the Unseen: Improved Estimators for Ent...

❌ MISSING DOIs

- 10.2307/j.ctv2rr3j2g.3 may be a valid DOI for title: BUB

❌ INVALID DOIs

- None
editorialbot commented 1 week ago

:wave: @openjournals/csism-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/6113, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

danielskatz commented 1 week ago

@kellino - I've suggested a number of small changes in https://github.com/kellino/DiscreteEntropy.jl/pull/17. Please merge this, or let me know what you disagree with.

Also, I note that there is no Acknowledgments section in your paper. Should there be (to acknowledge funding or other support)?

kellino commented 1 week ago

The only financial support was my salary, but that should probably be mentioned. I'll put that in now