openjournals / joss-reviews

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

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

Closed whedon closed 4 years ago

whedon commented 4 years ago

Submitting author: @MrMinimal64 (Jannik Michelfeit) Repository: https://github.com/MrMinimal64/multivar_horner Version: v.2.0.0 Editor: @dpsanders Reviewers: @henrik227, @alex-konovalov Managing EiC: Arfon Smith

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

Author instructions

Thanks for submitting your paper to JOSS @MrMinimal64. Currently, there isn't an JOSS editor assigned to your paper.

@MrMinimal64 if you have any suggestions for potential reviewers then please mention them here in this thread (without tagging them with an @). In addition, this list of people have already agreed to review for JOSS and may be suitable for this submission (please start at the bottom of the list).

Editor instructions

The JOSS submission bot @whedon is here to help you find and assign reviewers and start the main review. To find out what @whedon can do for you type:

@whedon commands
whedon commented 4 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks.

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

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
Software report (experimental):

github.com/AlDanial/cloc v 1.84  T=0.07 s (498.1 files/s, 70070.0 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          15            764            967           1832
reStructuredText                12            419            168            389
TeX                              2             46              0            325
Markdown                         1             55              0            100
YAML                             3             20             17             37
INI                              1              9              1             22
make                             1              4              7              9
Bourne Shell                     2              5              0              9
-------------------------------------------------------------------------------
SUM:                            37           1322           1160           2723
-------------------------------------------------------------------------------

Statistical information for the repository '2224' was gathered on 2020/05/22.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
MrMinimal64                     38          8971           5410          100.00

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
MrMinimal64                3563           39.7          6.0               17.32
whedon commented 4 years ago

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

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.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
arfon commented 4 years ago

:wave: @MrMinimal64 - thanks for your submission to JOSS. Could you please add a few sentences to your paper to help those less intensely involved in mathematics understand the downstream applications of this work? That would help to assess the impact of the effort and help clarify the range of research users who would be impacted by this.

Also, as described in our blog post announcing the reopening of JOSS, we're currently working in a "reduced service mode", limiting the number of papers assigned to any individual editor.

Since reopening JOSS earlier in the week we've had > 50 papers submitted and as such, yours has been put in our backlog that we will be working through over the coming weeks and months.

Thanks in advance for your patience!

jannikmi commented 4 years ago

Hello @arfon, thanks for your quick response. Could you tell me which parts I should clarify or which parts are unclear? I thought I had described the advantages and potential use cases of the package extensively in the first section "Summary". The following sections give a more detailed explanation of the implemented approach and underpin the advantages with benchmarks.

I am looking forward to your review.

jannikmi commented 4 years ago

should I suggest reviewers myself?

arfon commented 4 years ago

Hello @arfon, thanks for your quick response. Could you tell me which parts I should clarify or which parts are unclear?

I think a little more detail in the summary section would be useful. For example, are there applied research fields where the factorisation of multivariate polynomials is a key aspect of the research work/analysis?

should I suggest reviewers myself?

Yes, please go ahead but as I mentioned above, we won't actually begin the review process until an editor is assigned to this paper which may be some time due to a large backlog of submissions.

jannikmi commented 4 years ago

I clarified the possible application of this package in the paper and added example applications where the package is already being used.

jannikmi commented 4 years ago

Based on the topics (applied mathematics, numerical analysis...) and preferred language (Python) they provided, I suggest the following reviewers:

munkm
SebastianoF
diehlpk
crstnbr
mstimberg
cpgr
fccoelho
bonh
dglmoore
jannikmi commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

PDF failed to compile for issue #2224 with the following error:

Error reading bibliography ./paper.bib (line 72, column 3): unexpected "y" expecting space, ",", white space or "}" Error running filter pandoc-citeproc: Filter returned error status 1 Looks like we failed to compile the PDF

jannikmi commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

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

danielskatz commented 4 years ago

👋 @dpsanders - would you be able to edit this submission?

danielskatz commented 4 years ago

@whedon invited @dpsanders as editor

whedon commented 4 years ago

I'm sorry human, I don't understand that. You can see what commands I support by typing:

@whedon commands
danielskatz commented 4 years ago

@whedon invite @dpsanders as editor

whedon commented 4 years ago

@dpsanders has been invited to edit this submission.

dpsanders commented 4 years ago

@danielskatz Yes sure.

@whedon assign @dpsanders as editor

dpsanders commented 4 years ago

@whedon assign @dpsanders as editor

whedon commented 4 years ago

OK, the editor is @dpsanders

dpsanders commented 4 years ago

@whedon check repository

whedon commented 4 years ago
Software report (experimental):

github.com/AlDanial/cloc v 1.84  T=0.17 s (215.5 files/s, 30338.0 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          15            761            961           1829
reStructuredText                12            422            168            391
TeX                              2             46              0            332
Markdown                         1             53              0            106
YAML                             3             20             17             37
INI                              1              9              1             22
make                             1              4              7              9
Bourne Shell                     2              5              0              9
-------------------------------------------------------------------------------
SUM:                            37           1320           1154           2735
-------------------------------------------------------------------------------

Statistical information for the repository '2224' was gathered on 2020/05/28.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
MrMinimal64                     39          8975           5426          100.00

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
MrMinimal64                3551           39.6          6.8               17.21
dpsanders commented 4 years ago

👋 Hi @MrMinimal64. Thanks for submitting to JOSS!

I have been assigned as the editor for your submission. Looking over it, I have a few questions that I would like you to address in the paper / docs. They are a mixture of technical and about the impact of the submission.

Thanks!

dpsanders commented 4 years ago

Also, a comparison with the Julia package https://github.com/JuliaAlgebra/StaticPolynomials.jl may be interesting, since that seems very relevant / related.

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,

Thank you for your detailed review. I tried to address all of your open questions and remarks in the latest commit. Please have a look at the new version of the paper. Some points I could however not resolve:

There are several representations of multivariate polynomials. Does your package specify or require one?

The core is “specifying” a Horner factorisation representation, but this “requires” the input of a canonical representation. Is this evident from the paper?

Are you basically providing an implementation of multivariate polynomial manipulation?

In my understanding polynomial manipulation is about computations with polynomials, like addition etc.. This is not supported by multivar_horner. Consequently it is also not being mentioned in the paper or documentation. Should I specifically state that this is not within the scope of the package?

Can you define more precisely […] the advantages that the [Horner factorisation] has?

I stated in the paper that the Horner factorisation has a lower numerical error and requires less operations during an evaluation. The included figures try to prove these claims. Do you have suggestions on how to define the advantages more precisely?

It seems that the main application may be to multivariate interpolation. […] The JOSS editors requested that you add a paragraph […] to clarify the range of research users who would be impacted.

The functionality of this package is computing an advantageous representation of multivariate polynomials. Since this contribution is so general, it is hard, and would in my opinion even be misleading, to give specific use cases (apart from examples).

The JOSS editors requested that you add a paragraph to help a more general audience understand the impact and applications of your work […].

What aspects of the approach remain unclear (for a more general audience)? The way I see it, an explanation for a wider audience would require the definition of very basic mathematical concepts like polynomials etc.. For the intended audience (= potential users of the presented package) some basic knowledge about these concepts can, and i even think must, be assumed. Everything else would blow up the scope of the paper and also obscure the relevant information for the intended audience. Would it not? Please correct me if I am routine-blinded and some aspects of the presented approach are incomprehensible even to readers with prior knowledge.

Some of the figures do not need to be so large

How do I adjust the width of the figures? It seems to me that the markdown standard itself does not specify this.

What are the units for the error in figure 3?

Since the given values are relative (to the numerical error of evaluation with Horner factorisation), there is no unit.

Do you have an implementation of this [multivariate interpolation]?

Yes, there is an implementation, but it is work in progress and not public (yet). Feel free to contact me if you are interested.

Thanks again for your review.

jannikmi commented 4 years ago

Now changes are in the master branch.

@whedon generate pdf

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 added some clarifications and examples of canonical form polynomial representations.

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

Are there any suggestions?

dpsanders commented 4 years ago

Sorry for the delay, I will be available again at the end of the week.

dpsanders commented 4 years ago

👋 @MrMinimal64, I think you've successfully addressed all my questions and comments, thanks!

dpsanders commented 4 years ago

👋 @moorepants, @alex-konovalov and @henrik227, would any of you be willing to review this submission for JOSS? We carry out our checklist-driven reviews here in GitHub issues and follow these guidelines: https://joss.readthedocs.io/en/latest/review_criteria.html. Thanks!

hbarthels commented 4 years ago

Yes, I can do that.

dpsanders commented 4 years ago

@whedon assign @henrik227 as reviewer

whedon commented 4 years ago

OK, @henrik227 is now a reviewer

olexandr-konovalov commented 4 years ago

@dpsanders thanks, I can do that too.

dpsanders commented 4 years ago

@whedon add @alex-konovalov as reviewer

whedon commented 4 years ago

OK, @alex-konovalov is now a reviewer

dpsanders commented 4 years ago

@whedon start review

whedon commented 4 years ago

OK, I've started the review over in https://github.com/openjournals/joss-reviews/issues/2392.

dpsanders commented 4 years ago

Thanks @henrik227 and @alex-konovalov! In #2392 there are checklists for each of you to guide the review process. Please let me know if you have any questions or comments.