openjournals / joss-reviews

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

[REVIEW]: CBX: Python and Julia Packages for Consensus-Based Interacting Particle Methods #6611

Closed editorialbot closed 4 months ago

editorialbot commented 7 months ago

Submitting author: !--author-handle-->@rafaelbailo<!--end-author-handle-- (Rafael Bailo) Repository: https://github.com/PdIPS/CBX Branch with paper.md (empty if default branch): Version: v0.1.6-v1.3.1 Editor: !--editor-->@mstimberg<!--end-editor-- Reviewers: @AlessandroPierro, @gdalle, @kellertuer, @Bobby-Huggins Archive: 10.5281/zenodo.12207224

Status

status

Status badge code:

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

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

@AlessandroPierro & @gdalle & @kellertuer & @Bobby-Huggins, 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 @mstimberg 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 @AlessandroPierro

📝 Checklist for @gdalle

📝 Checklist for @kellertuer

📝 Checklist for @Bobby-Huggins

editorialbot commented 7 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 7 months ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.01 s (545.3 files/s, 84623.3 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
TeX                              1             52              0            524
Markdown                         2             39              0            121
YAML                             1              1              4             18
JSON                             1              0              0             17
-------------------------------------------------------------------------------
SUM:                             5             92              4            680
-------------------------------------------------------------------------------

Commit count by author:

    17  Dr Rafael Bailo
    16  Tim Roith
    10  Konstantin Riedl
     5  TimRoith
     5  ctotzeck
     4  Urbain Vaes
     2  Susana Gomes
     2  aletheaBarbaro
editorialbot commented 7 months ago

Paper file info:

📄 Wordcount for paper.md is 1868

✅ The paper includes a Statement of need section

editorialbot commented 7 months ago

License info:

🔴 Failed to discover a valid open source license

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

OK DOIs

- 10.1038/s41586-020-2649-2 is OK
- 10.1038/s41592-019-0686-2 is OK
- 10.1109/MCSE.2007.55 is OK
- 10.21105/joss.05320 is OK
- 10.5281/zenodo.1054110 is OK
- 10.21105/joss.03866 is OK
- 10.5281/zenodo.7754236 is OK
- 10.1142/S0218202518500276 is OK
- 10.1017/S0956792523000293 is OK
- 10.1142/S0218202521500342 is OK
- 10.5281/ZENODO.7738525 is OK
- 10.1007/978-3-662-12616-5 is OK
- 10.1142/s0218202517400061 is OK
- 10.1007/978-3-662-38527-2_55 is OK
- 10.1016/j.eswa.2011.09.076 is OK
- 10.1007/s10462-012-9328-0 is OK
- 10.1007/978-3-642-04944-6_14 is OK
- 10.1007/978-1-4613-1997-9 is OK
- 10.21105/joss.00433 is OK
- 10.21105/joss.00615 is OK
- 10.21105/joss.04723 is OK
- 10.1051/cocv/2020046 is OK
- 10.1007/978-3-031-02462-7_46 is OK
- 10.1007/s10898-024-01369-1 is OK

MISSING DOIs

- No DOI given, and none found for title: Studien ueber das Gleichgewicht der lebenden Kraft
- No DOI given, and none found for title: The wind driven optimization technique and its app...
- No DOI given, and none found for title: Particle swarm optimization
- No DOI given, and none found for title: The theory and practice of simulated annealing
- No DOI given, and none found for title: The convergence of the random search method in the...
- No DOI given, and none found for title: Planning experiments seeking maxima
- No DOI given, and none found for title: "Direct Search" Solution of Numerical and Statisti...
- No DOI given, and none found for title: Consensus-based optimization methods converge glob...
- No DOI given, and none found for title: Trends in consensus-based optimization
- No DOI given, and none found for title: Consensus-based sampling
- No DOI given, and none found for title: scikit-opt
- No DOI given, and none found for title: DEAP: Evolutionary Algorithms Made Easy
- No DOI given, and none found for title: cbo-in-python
- No DOI given, and none found for title: polarcbo
- No DOI given, and none found for title: PyPop7: A Pure-Python Library for Population-Based...
- No DOI given, and none found for title: Polarized consensus-based dynamics for optimizatio...
- No DOI given, and none found for title: Pytorch: An imperative style, high-performance dee...
- No DOI given, and none found for title: On the global convergence of particle swarm optimi...
- No DOI given, and none found for title: Gradient is All You Need?
- No DOI given, and none found for title: FedCBO: Reaching Group Consensus in Clustered Fede...
- No DOI given, and none found for title: An adaptive consensus based method for multi-objec...
- No DOI given, and none found for title: Consensus-based optimization on the sphere: conver...
- No DOI given, and none found for title: Constrained consensus-based optimization
- No DOI given, and none found for title: On the mean-field limit for the consensus-based op...
- No DOI given, and none found for title: Consensus-based optimization for saddle point prob...
- 10.1017/s095679252400007x may be a valid DOI for title: Consensus-Based Optimization with Truncated Noise
- No DOI given, and none found for title: Large deviations techniques and applications
- No DOI given, and none found for title: Consensus-based rare event estimation

INVALID DOIs

- None
editorialbot commented 7 months ago

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

mstimberg commented 7 months ago

👋🏼 @rafaelbailo @AlessandroPierro, @gdalle, @kellertuer, @Bobby-Huggins 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.

There are additional guidelines in the message at the start of this issue.

Please feel free to ping me (@mstimberg) if you have any questions/concerns.

Please note that the repository linked above only contains the JOSS paper. The actual code is in

Since the JOSS machinery does not directly support this setup, please ignore the "Software report" with the lines of code above, and the "Failed to discover a valid open source license" statement (both packages are distributed under the OSI-approved MIT license).

rafaelbailo commented 7 months ago

Thank you @mstimberg for setting everything up, and thanks in advance to the reviewers for their time!

kellertuer commented 7 months ago

Review checklist for @kellertuer

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

gdalle commented 7 months ago

Review checklist for @gdalle

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

AlessandroPierro commented 6 months ago

Review checklist for @AlessandroPierro

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Bobby-Huggins commented 6 months ago

Review checklist for @Bobby-Huggins

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

kellertuer commented 6 months ago

I have a short – more like organisational – question on one of the points to check, concerning

Repository: Is the source code for this software available at the https://github.com/PdIPS/CBX?

The correct answer is: No. That is (just) the JOSS paper. But it is hard to specify what to link to since this paper concerns two software packages (https://github.com/PdIPS/CBXpy and https://github.com/PdIPS/ConsensusBasedX.jl) such that I feel the question is maybe in general not fitting completely here (unless one allows for two urls there).

mstimberg commented 6 months ago

@kellertuer I agree that the situation is not ideal here, but unfortunately the JOSS system only allows for a single URL in a submission. Having a third repository with the JOSS paper in it was a suggestion by the editor-in-chief, so I think it is the best we can do for now. To make the checklists less ambiguous, I edited them to refer to the repository for either CBXpy or ConsensusBasedX.jl (in case you haven't seen it, I mentioned the URLs in my earlier comment).

@rafaelbailo I noticed that the README at https://github.com/PdIPS/CBX only links to the documentation pages for CBXpy and ConsensusBasedX.jl, could you please also include links to their GitHub repositories so that future visitors can easily find the source code?

kellertuer commented 6 months ago

Sure, I of course found both easily, then I was maybe taking the question too rigorous (sorry, a German following rules here! ;) ). I also think that the third repo with the paper is the best way when the paper is about two packages. And with linking them in the readme I am also fine then.

mstimberg commented 6 months ago

Sure, I of course found both easily, then I was maybe taking the question too rigorous (sorry, a German following rules here! ;) ).

That's why I edited the checklist so that you can now check the box without breaking any rules :stuck_out_tongue_closed_eyes:

TimRoith commented 6 months ago

@kellertuer I agree that the situation is not ideal here, but unfortunately the JOSS system only allows for a single URL in a submission. Having a third repository with the JOSS paper in it was a suggestion by the editor-in-chief, so I think it is the best we can do for now. To make the checklists less ambiguous, I edited them to refer to the repository for either CBXpy or ConsensusBasedX.jl (in case you haven't seen it, I mentioned the URLs in my earlier comment).

@rafaelbailo I noticed that the README at https://github.com/PdIPS/CBX only links to the documentation pages for CBXpy and ConsensusBasedX.jl, could you please also include links to their GitHub repositories so that future visitors can easily find the source code?

Thanks for pointing this out. The ReadMe of the paper now points to the repositories, instead of the documentation pages.

gdalle commented 6 months ago

I have completed my (Julia-specific) review, both the paper and package seem very high quality to me. I have made a few remarks in https://github.com/PdIPS/ConsensusBasedX.jl/issues/31, but my two main issues are:

AlessandroPierro commented 6 months ago

My review, focused on the Python side, is complete. The contribution is well-polished and checks all JOSS requirements. I had a minor comment on contributing guidelines (ref. PdIPS/CBXpy/issues/23) and the authors promptly addressed it.

@rafaelbailo, thank you for your submission!

mstimberg commented 5 months ago

Many thanks for your reviews @gdalle and @AlessandroPierro :pray:

@kellertuer and @Bobby-Huggins, could you give us an estimate when you can have a detailed looked at the software/paper?

kellertuer commented 5 months ago

I am waiting on comments from the authors in my open issue linked above.

mstimberg commented 5 months ago

I am waiting on comments from the authors in my open issue linked above.

My apologies, I completely overlooked this issue – the ball is in @rafaelbailo's court, then :blush:

Bobby-Huggins commented 5 months ago

And I expect to be able to complete my review this evening (Central US timezone).

Bobby-Huggins commented 5 months ago

My comments on the paper and Python package are here: https://github.com/PdIPS/CBXpy/issues/28

Overall excellent submission: I just echo @gdalle in mentioning that it looks like there could be more test coverage on the actual optimization results (but test coverage overall is great). I also mention one apparent bug in the plotting routine, and some small comments on the paper.

mstimberg commented 5 months ago

Thanks a lot for the update, @Bobby-Huggins.

Bobby-Huggins commented 5 months ago

My comments have been addressed by the authors to my satisfaction, and I've checked off the last items on my checklist.

mstimberg commented 5 months ago

:wave: @rafaelbailo and @TimRoith, could you give us an estimate when you'll be able to address the remaining comments by the reviewers? As fars I can tell, PdIPS/ConsensusBasedX.jl#31 and PdIPS/CBX#3 are still waiting for changes/replies/clarifications. Thanks!

gdalle commented 5 months ago

Yes indeed, thanks for answering my remarks about the paper in https://github.com/PdIPS/ConsensusBasedX.jl/issues/31, but my main concerns remain about the documentation and the tests.

rafaelbailo commented 5 months ago

Hi @mstimberg, we've just implemented the changes required in PdIPS/ConsensusBasedX.jl#31 and PdIPS/CBX#3. Thank you for your patience (and to @kellertuer and @gdalle)!

mstimberg commented 5 months ago

Many thanks for the update, @rafaelbailo.

@gdalle, @kellertuer, please take a look whether the changes address your remaining concerns.

rafaelbailo commented 5 months ago

@mstimberg I have a question. I've just changed affiliations, and I'm unsure of which affiliation to list on the paper. Is the JOSS policy to list the affiliation where most of the work was done, the most recent affiliation, or both?

mstimberg commented 5 months ago

@rafaelbailo I am not aware that JOSS has a specific policy in that regard, meaning that it is up to you (or e.g. your employer's policy). But I will check back with the other editors to be sure.

mstimberg commented 5 months ago

Hi again @rafaelbailo. I checked back with the other editors, and there is indeed no specific policy from JOSS's side about this. In some fields, it would be most common to list the affiliation where you did your work (and maybe add a footnote about your changed affiliation). But others would consider it most common to use the current affiliation (and potentially add a footnote in the other sense). Note that in either case, if you keep your ORCID page up to date, both affiliations will be linked via it. So to sum up: please use your best judgement, we are not imposing either convention from our side :blush:

rafaelbailo commented 5 months ago

Hi @mstimberg, thank you very much for double-checking, this is helpful!

mstimberg commented 4 months ago

:wave: @gdalle, @kellertuer, could you please give us an estimate when you will be able to verify whether the latest changes address all your concerns?

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

gdalle commented 4 months ago

@editorialbot commands

editorialbot commented 4 months ago

Hello @gdalle, here are the things you can ask me to do:


# List all available commands
@editorialbot commands

# Get a list of all editors's GitHub handles
@editorialbot list editors

# Adds a checklist for the reviewer using this command
@editorialbot generate my checklist

# Set a value for branch
@editorialbot set joss-paper as branch

# Run checks and provide information on the repository and the paper file
@editorialbot check repository

# Check the references of the paper for missing DOIs
@editorialbot check references

# Generates the pdf paper
@editorialbot generate pdf

# Generates a LaTeX preprint file
@editorialbot generate preprint

# Get a link to the complete list of reviewers
@editorialbot list reviewers
gdalle commented 4 months ago

@editorialbot check references

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

OK DOIs

- 10.1109/TAP.2013.2238654 is OK
- 10.5281/zenodo.2559634 is OK
- 10.1109/ICEC.1996.542381 is OK
- 10.1109/TEVC.2009.2029567 is OK
- 10.1109/ICNN.1995.488968 is OK
- 10.1007/0-306-48056-5_10 is OK
- 10.1145/321062.321069 is OK
- 10.1007/978-3-030-93302-9_6 is OK
- 10.1111/sapm.12470 is OK
- 10.1038/s41586-020-2649-2 is OK
- 10.1038/s41592-019-0686-2 is OK
- 10.1109/MCSE.2007.55 is OK
- 10.21105/joss.00431 is OK
- 10.21105/joss.05320 is OK
- 10.5281/zenodo.1054110 is OK
- 10.21105/joss.03866 is OK
- 10.5281/zenodo.7754236 is OK
- 10.21105/joss.05694 is OK
- 10.1142/S0218202518500276 is OK
- 10.1017/S0956792523000293 is OK
- 10.1007/s00245-023-09983-3 is OK
- 10.1142/S0218202521500342 is OK
- 10.1137/22M1471304 is OK
- 10.1002/mma.8279 is OK
- 10.1137/22M1543367 is OK
- 10.1017/S095679252400007X is OK
- 10.1007/978-3-642-03311-7 is OK
- 10.5281/ZENODO.7738525 is OK
- 10.1007/978-3-662-12616-5 is OK
- 10.1142/s0218202517400061 is OK
- 10.1007/978-3-662-38527-2_55 is OK
- 10.1016/j.eswa.2011.09.076 is OK
- 10.1007/s10462-012-9328-0 is OK
- 10.1007/978-3-642-04944-6_14 is OK
- 10.1007/978-1-4613-1997-9 is OK
- 10.21105/joss.00433 is OK
- 10.21105/joss.00615 is OK
- 10.21105/joss.04723 is OK
- 10.1051/cocv/2020046 is OK
- 10.1007/978-3-031-02462-7_46 is OK
- 10.1007/s10898-024-01369-1 is OK

MISSING DOIs

- No DOI given, and none found for title: Studien ueber das Gleichgewicht der lebenden Kraft
- No DOI given, and none found for title: The convergence of the random search method in the...
- No DOI given, and none found for title: Planning experiments seeking maxima
- No DOI given, and none found for title: Consensus-based optimization methods converge glob...
- No DOI given, and none found for title: GPyOpt: A Bayesian Optimization framework in pytho...
- No DOI given, and none found for title: BoTorch: A Framework for Efficient Monte-Carlo Bay...
- No DOI given, and none found for title: GPflowOpt: A Bayesian Optimization Library using T...
- No DOI given, and none found for title: Bayesian Optimization: Open source constrained glo...
- No DOI given, and none found for title: scikit-opt
- No DOI given, and none found for title: DEAP: Evolutionary Algorithms Made Easy
- No DOI given, and none found for title: cbo-in-python
- No DOI given, and none found for title: polarcbo
- No DOI given, and none found for title: PyPop7: A Pure-Python Library for Population-Based...
- 10.1007/s10107-024-02095-y may be a valid DOI for title: Polarized consensus-based dynamics for optimizatio...
- No DOI given, and none found for title: Pytorch: An imperative style, high-performance dee...
- No DOI given, and none found for title: Gradient is All You Need?
- No DOI given, and none found for title: FedCBO: Reaching Group Consensus in Clustered Fede...
- No DOI given, and none found for title: Consensus-based optimization on the sphere: conver...
- 10.1137/23m1565966 may be a valid DOI for title: Consensus-based rare event estimation

INVALID DOIs

- doi.org/10.1007/s00245-023-10036-y is INVALID because of 'doi.org/' prefix
gdalle commented 4 months ago

Apart from the missing/invalid DOIs and some last remarks in https://github.com/PdIPS/ConsensusBasedX.jl/issues/31, this is good to go on my end

kellertuer commented 4 months ago

Besides the remarks from @gdalle, the only thing I noticed is that now the Julia and Python packages are inconsistent - one uses optimise (upon my request to not use 4 names for the same function) and the python one uses minimise for the same – has the Python package more names than that?

TimRoith commented 4 months ago

I guess right now, Python (only) uses optimize and Julia minimise.

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

kellertuer commented 4 months ago

If it really only defines that in Python that is fine with me.

gdalle commented 4 months ago

@TimRoith now Figure 1 is missing altogether. I guess that is one way of making it not blurry

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