[REVIEW]: multivar_horner: A Python package for computing Horner factorisations of multivariate polynomials

whedon commented 4 years ago

Submitting author: @MrMinimal64 (Jannik Michelfeit) Repository: https://github.com/MrMinimal64/multivar_horner Version: 2.1.1 Editor: @dpsanders Reviewers: @henrik227, @saschatimme Archive: 10.5281/zenodo.4062422

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

Status badge code:

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

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

@henrik227, @alex-konovalov and @saschatimme, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

Make sure you're logged in to your GitHub account
Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @dpsanders know.

✨ Please try and complete your review in the next six weeks ✨

Review checklist for @henrik227

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the repository url?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@MrMinimal64) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
[x] 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

Software paper

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

Review checklist for @saschatimme

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the repository url?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@MrMinimal64) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
[x] 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

Software paper

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

whedon commented 4 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @henrik227, @alex-konovalov it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf

whedon commented 4 years ago

Reference check summary:

OK DOIs

- 10.1109/MCSE.2011.37 is OK
- 10.1145/2833157.2833162 is OK

MISSING DOIs

- https://doi.org/10.1098/rstl.1819.0023 may be missing for title: XXI. A new method of solving numerical equations of all orders, by continuous approximation
- https://doi.org/10.1201/9781315120195-1 may be missing for title: Introduction to numerical analysis
- https://doi.org/10.1007/978-3-030-01177-2_5 may be missing for title: Fast Interpolation and Fourier Transform in High-Dimensional Spaces
- https://doi.org/10.1145/980175.980179 may be missing for title: Greedy algorithms for optimizing multivariate Horner schemes
- https://doi.org/10.1007/s006070070002 may be missing for title: On the multivariate Horner scheme II: running error analysis
- https://doi.org/10.15807/jorsj.51.29 may be missing for title: Efficient evaluation of polynomials and their partial derivatives in homotopy continuation methods
- https://doi.org/10.1090/s0025-5718-1990-0993925-1 may be missing for title: Evaluation of multivariate polynomials and their derivatives
- https://doi.org/10.1016/j.cpc.2013.05.008 may be missing for title: Improving multivariate Horner schemes with Monte Carlo tree search
- https://doi.org/10.1007/978-3-642-15582-6_55 may be missing for title: Efficient evaluation of large polynomials

INVALID DOIs

- None

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

jannikmi commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

jannikmi commented 4 years ago

@whedon set v2.1.0 as version

whedon commented 4 years ago

I'm sorry @MrMinimal64, I'm afraid I can't do that. That's something only editors are allowed to do.

jannikmi commented 4 years ago

@dpsanders please bump the sofware version to v2.1.0 (latest release). Some useful API changes/clarifications have been included: https://github.com/MrMinimal64/multivar_horner/blob/master/CHANGELOG.rst#210-2020-06-15

dpsanders commented 4 years ago

@MrMinimal64 I don't think we need to do anything about the versioning until the review is complete.

hbarthels commented 4 years ago

I am concerned that multivar_horner does not qualify as a `substantial scholarly effort' as described in the review criteria, for the following reasons:

The project is not very large (see the statistics here).
It seems to be a rather straight-forward implementation of an existing algorithm (described in Ceberio & Kreinovich, 2004).

However, I do believe that multivar_horner is useful for anyone who needs to evaluate large multivariate polynomials.

Initially, I thought that the search-based approach mention in the `further reading' section could be a major contribution of multivar_horner. However, according to the documentation, this approach does not seem to have any practical benefits (yet?).

jannikmi commented 4 years ago

hello @henrik227 and @dpsanders,

@henrik227, I do respect your opinion, but I would like to briefly argue against it in the following.

The project is not very large (see the statistics here).

How large a project needs to be in order to fulfil the JOSS review criteria is open for discussion. What can be said definitively however, is that many projects with a similar or even smaller footprint are being considered: https://github.com/openjournals/joss-reviews/issues/2428#issuecomment-653255875 https://github.com/openjournals/joss-reviews/issues/2438#issuecomment-653794412

It seems to be a rather straight-forward implementation of an existing algorithm (described in Ceberio & Kreinovich, 2004).

First of all I would like to point out that multivar_horner contains many computational aspects not included in the solely theoretical outline of the algorithm in (Ceberio & Kreinovich, 2004). The mathematical considerations only comprise the factorisation process. How a found Horner factorisation can be represented and evaluated in a computationally efficient way is not within the scope of the literature. Whether or not an implementation is “straight-forward”, in the end always remains highly subjective and even if it was, I think it could very well still qualify as “substantial scholarly effort”.

multivar_horner can be considered as a substantial scholarly effort due to the following reasons:

It is the first publicly available implementation of any kind of Horner factorisations for multivariate polynomials. Without it, the, as you call them, "existing algorithms" were existing actually only in theory or private and in absence of any benchmarks, tests and documentation.
Because of their simple differentiation and integration, as well as their pleasant vector space structure, polynomials in multiple variables are a standard choice as interpolants and are fundamental in ordinary differential equation (ODE) and partial differential equation (PDE) solvers (see for example [1-4]).
Horner factorisations have beneficial properties for handling polynomials as I am proving in the paper under review here
How well all the theoretical approaches mentioned above can be “substantial” to academia depends on transferring them into practice by providing numerical availablility and applicability. That is exactly what this package does, which in turn makes it “substantial” to academia as well.
That the package has been downloaded 11k times in total and on average 27 times a day in the last month (s. https://pepy.tech/project/multivar-horner) demonstrates that there is a demand for the project and its code quality is adequate

As an open source package helpful for a large part of the scientific community multivar_horner is a valid candidate for being published in the JOSS.

[1] Floater, Michael S., and Kai Hormann. "Barycentric rational interpolation with no poles and high rates of approximation." Numerische Mathematik 107.2 (2007): 315-331. [2] Cools, Ronald. "Constructing cubature formulae: the science behind the art." Acta numerica 6 (1997): 1-54. [3] Temlyakov, Vladimir N. "Cubature formulas, discrepancy, and nonlinear approximation." Journal of Complexity 19.3 (2003): 352-391. [4] Cools, Ronald. "Advances in multidimensional integration." Journal of computational and applied mathematics 149.1 (2002): 1-12.

hbarthels commented 4 years ago

To clarify: I do not question that the package is useful. However, I cannot say for sure that this package "represent[s] not less than three months of work for an individual", and from the review criteria it is not clear to me how the usefulness of the software is weighed against the effort that went into it. I am well aware that it is difficult to objectively quantify the development effort of software.

I do not have a strong opinion on this; I just want to make sure that the package meets the standards of JOSS. If @dpsanders and @alex-konovalov consider the package a "substantial scholarly effort", I am ok with that.

dpsanders commented 4 years ago

@henrik227 Thanks for expressing your concern.

I am waiting for @alex-konovalov's input, and for a further opinion which I hope to get next week.

dpsanders commented 4 years ago

@whedon add saschatimme as reviewer

whedon commented 4 years ago

OK, saschatimme is now a reviewer

dpsanders commented 4 years ago

@whedon add @saschatimme as reviewer

whedon commented 4 years ago

OK, @saschatimme is now a reviewer

dpsanders commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

jannikmi commented 4 years ago

in the meantime a preprint version has been published on arXiv.org

jannikmi commented 4 years ago

The current paper version now contains some additional aspects:

an abstract also used to the arXiv paper version
mentioning of Maple as a software already supporting multivariate Horner factorisations
reference to Mathematica offering 1D Horner factorisations
mention the ability to a posteriori change the coefficients of a factorised polynomial (the multivar_horner factorisation process is coefficient agnostic)

@saschatimme, @alex-konovalov, what is the status of the review?

jannikmi commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

jannikmi commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

jannikmi commented 4 years ago

Hello @dpsanders, @saschatimme, @alex-konovalov, @henrik227, This review already started 2 month ago. What is the current status and the timeline for the next steps? With kind regards, Jannik Michelfeit

dpsanders commented 4 years ago

👋 @MrMinimal64, apologies for my absence.

👋 @henrik227, @alex-konovalov, @saschatimme: How are your reviews of this submission coming along? Hopefully we can have a first round of comments finished, say, by the end of August? Thanks!

hbarthels commented 4 years ago

@dpsanders I was waiting on a decision regarding whether this project is considered a "substantial scholarly effort". Wouldn't it make sense to decide on that first?

dpsanders commented 4 years ago

@henrik227 That's a fair point. In my opinion this submission does meet the criteria for a "substantial scholarly effort" and should be reviewed.

jannikmi commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

jannikmi commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

jannikmi commented 4 years ago

I tried to incorporate all the objections made by the reviewers in the latest version of the paper.

jannikmi commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

dpsanders commented 4 years ago

👋 @alex-konovalov: Will you be able to complete your review of this paper soon?

dpsanders commented 4 years ago

👋 @saschatimme and @henrik227: Many thanks for your very useful feedback in the issues on the repo! Are you happy with the current version?

jannikmi commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

hbarthels commented 3 years ago

👋 @saschatimme and @henrik227: Many thanks for your very useful feedback in the issues on the repo! Are you happy with the current version?

I still disagree with how operations are counted in this package, but I think it's ok since it's explained both in the paper and the documentation.

saschatimme commented 3 years ago

I am in the same boat as @henrik227 that I disagree with the way operations are counted in the package. The article states

multivar_horner implements a multivariate Horner scheme using the greedy heuristic presented in (Ceberio & Kreinovich, 2004).

However, looking into (Ceberio & Kreinovich, 2004) the way operations are counted there is actually how @henrik227 and I suggest they are supposed to be counted (to compute x^3 takes 2 operations) and not as the author implemented it and describes it in the article where x^3 only requires single operation.

jannikmi commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

jannikmi commented 3 years ago

The operations are now being counted as you suggest (solely the number of multiplications). The optimal search heuristic now minimises the new operation count and finds "optimal" factorisations in this sense. I also evaluate the relative numerical error in the benchmarks. The paper as well as the documentation have been updated accordingly.

jannikmi commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

jannikmi commented 3 years ago

I updated the placement of the figures (now the paper has a length of 8 pages again)

jannikmi commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

openjournals / joss-reviews