[JOSS] Paper review 1

[matbesancon]

You will find below my initial review of the paper itself:

Title of the package: Why "Algebraic"? It uses numerical methods and not symbolic differentiation.

First line: use \mathbb{R} instead of \mathcal{R}

Prefer Package.jl to Package to designate it

Second paragraph of "Summary" "adjustment of mathematical functions to data" is also refered to as regression, which is not mentioned in the paper, while it is fundamentally the purpose of the package.

"Detection of outliers is always regarded ...". It's hard to see what the authors mean here. Without proper references backing up the statement, this seems rather vague.

"Recently, a good review", the use of "good" is subjective (what is a good review, in what context). A review of what?

In statistics, it is common to use f(x; θ) for a function of x with parameters theta, which one tries to fit in a regression task.

In paragraph "Functionality", the equation is not homogeneous with the rest: ϕ(x,t) instead of ϕ(x;t), prefer ";" to indicate the separation of data and parameters.

There could be a section on the method itself, or at least more development, including on the LOVO algorithm. For instance, the method is not black-box optimization but assumes the existence of gradient, thus making it first-order. Furthermore, if gradients are not provided by users, the use of ForwardDiff makes it mandatory to have the function defined in a generic way (accepting any Real number). Thus, how is the proposed method different from using other first-order methods (such as the ones in Optim.jl or JuliaSmoothOptimizers)? This should be detailed.

"it is obvious that its arguments [...]", avoid the use of "obvious" which is subjective, and develop what is meant here.

"Additional features" paragraph, sentence: "the distributed version is a centralized ..." is a contradiction and should be re-phrased.

This belongs to the JOSS review https://github.com/openjournals/joss-reviews/issues/1385

[evcastelani]

1. [x] Title of the package: Why "Algebraic"? It uses numerical methods and not symbolic differentiation.

   The term "algebraic" refers to the type of problem solved and not to the programming techniques of the algorithm. Although it is not a common term in adjustment functions, it serves to differ from the geometric fit (geometrical fit functions). In the case of geometric fit, we are interested in minimizing the sum of deviations of the type min_x || (x, phi (t, x)) - (t_i, y_i) || (ie, orthogonal projection on the curve to be adjusted). In the algebraic case, we are interested in minimizing the sum of deviations of type || phi (t_i, x) - y_i ||. We chose this term to make the package's name more readable. This information has been added in the Introduction.

2. [x] First line: use \mathbb{R} instead of \mathcal{R}

3. [x] Prefer Package.jl to Package to designate it

4. [x] Second paragraph of "Summary". "adjustment of mathematical functions to data" is also referred to as regression, which is not mentioned in the paper, while it is fundamentally the purpose of the package.

   We have added the term regression in the manuscript.

5. [x] "Detection of outliers is always regarded ...". It's hard to see what the authors mean here. Without proper references backing up the statement, this seems rather vague.

   We have added to the second paragraph more information about techniques to deal with outlier detection. Also, we have described the impact of outliers when regression is performed by the sum-of-squares technique.

6. [x] "Recently, a good review", the use of "good" is subjective (what is a good review, in what context). A review of what?

   TODO: @evcastelani

7. [x] In statistics, it is common to use f(x; θ) for a function of x with parameters theta, which one tries to fit in a regression task.

   We have changed the whole package to adopt this statistical way of using. Now x refers to the model's variables and θ to its parameters, which we actually want to find.

8. [x] In paragraph "Functionality", the equation is not homogeneous with the rest: ϕ(x,t) instead of ϕ(x;t), prefer ";" to indicate the separation of data and parameters.

   We did not use ; in the code, as ; is used for optional parameters in Julia. We have used \phi(x, \theta) in the whole manuscript, so the model can be written in the same way in Julia.

9. [x] There could be a section on the method itself, or at least more development, including on the LOVO algorithm. For instance, the method is not black-box optimization but assumes the existence of gradient, thus making it first-order. Furthermore, if gradients are not provided by users, the use of ForwardDiff makes it mandatory to have the function defined in a generic way (accepting any Real number). Thus, how is the proposed method different from using other first-order methods (such as the ones in Optim.jl or JuliaSmoothOptimizers)? This should be detailed.

   1. New section defining the LOVO problem.
   2. We tried to make it clear the difference between derivatives of the model and the derivatives of the LOVO function
   3. For solving LOVO problems, there are first-order or second-order algorithms [ref Yano]. RAFF.jl implements a Levenberg-Marquardt algorithm (which is first-order) in the context of LOVO problems. This implementation is a novel algorithm. LOVO problems need the number of possible outliers to be given by the user. RAFF.jl solves this limitation by implementing a voting system. A voting system is a brute force algorithm, where several LOVO problems are solved with different numbers of possible outliers. The solution which most occurs among them is declared as the solution by RAFF.jl.

   This discussion has been added in the end of the Background section.

10. [x] "it is obvious that its arguments [...]", avoid the use of "obvious" which is subjective, and develop what is meant here.

    This phrase has been removed, since we replaced the example by a more intuitive one.

11. [x] "Additional features" paragraph, sentence: "the distributed version is a centralized ..." is a contradiction and should be re-phrased.

    Fixed. The distributed version is a master-slave implementation.

[matbesancon]

@evcastelani thanks for the answer list. For the algebraic part, putting this explanation and/or references in the paper would be useful

[matbesancon]

ping me when the editions are done

[abelsiqueira]

Review

The authors implement a solver for data fitting that automatically detects outliers, based on algorithms using Lower Order-Value Optimization. The package meets the criteria for publication on JOSS, but there are a few items that should be addressed, and a few suggestions.

• What kind of optimal fitting is achieved? Seems to be least squares (for the data without the outliers), but it's worth mentioning in the summary;
• In what sense are the outliers defined? Some quantile?
• Since the problem in the example is nonlinear, it should be indicated which solver was used to solve the problem, and which starting point;
• I think that a comparison with a different fitting not so sensitive to outliers would improve the quality of the paper. Perhaps fitting with norm-1 or Huber loss.
• The releases are not being registered. The Julia register has 0.4.3, but there is a release 0.6.0 already and the docs are being updated according to the releases;
• In the first paragraph of Additional features: "...provided, what results in...", change "what" to "which".
• On Figure 1, move the legend to the right, use a stronger color for the data points that are not outliers, and make it black-and-white/color-blind friendly by using a different marker style for the outliers and changing line-styles (I'd remove the "True solution").

Suggestions: These can be disregarded it the authors prefer:

• In the context of regression, I find that it's clearer to use x and y for the data and \beta or \theta for the parameters being optimized. Our traditional least squares problem min |Ax - b|^2 is usually written as min |Xβ - y|^2.
• I find that entering the data separately for x and y is better, e.g.

  x = range(-1, 1, length=100)
  y = 2 .+ 3x + randn(100) * 0.1
  model(β, x) = β[1] + β[2] * x[1]
  output = raff(model, x, y, 2) # Here

[fsobral]

@abelsiqueira , Thank you for your comments. Here are the answers for your suggestions. Most of them have been included in the new version of the manuscript.

• [x] What kind of optimal fitting is achieved? Seems to be least squares (for the data without the outliers), but it's worth mentioning in the summary;

  We have added an explanation regarding the kind of optimal fit that RAFF.jl finds in the Introduction. It is in the same sense as the traditional nonlinear regression or least squares.

• [x] In what sense are the outliers defined? Some quantile?

  Outliers are defined as the points for which the deviation to the model are the highest. RAFF.jl uses a voting system to try to detect the number of points that can be considered as outliers. To do this, it uses the ideas behind LOVO problems. A more formal description of those problems has been added in the Background section.

• [x] Since the problem in the example is nonlinear, it should be indicated which solver was used to solve the problem, and which starting point;

  We have replaced the example, since it was causing confusion about the problem that is solved. We hope that the section Background and the new example will make it clear the type of problem that we solve. We have also highlighted the fact that we implement a novel algorithm for LOVO problems, based on the classical Levenberg-Marquardt algorithm. LOVO problems cannot be solved by any nonlinear algorithm and we expected to achieve second-order convergence with this approach.

• [x] I think that a comparison with a different fitting not so sensitive to outliers would improve the quality of the paper. Perhaps fitting with norm-1 or Huber loss.

  We do not think that the 1-norm would be more robust than the least squares (suppose that the errors are smaller than 1, for example). In addition, the problem would become non-differentiable and harder to solve. Moreover, one can always create a sufficiently far outlier so that the "more robust" approaches also will construct poor models. Our new example was intended to show how could one benefit for detecting and eliminating outliers, which is different from simply tolerating them. Although it is easy to see the two outliers, when the dimension of the problem increases it becomes impossible to visually detect and remove them.

• [x] The releases are not being registered. The Julia register has 0.4.3, but there is a release 0.6.0 already and the docs are being updated according to the releases;

  We have submitted the two new versions of RAFF.jl to registration.

• [x] In the first paragraph of Additional features: "...provided, what results in...", change "what" to "which".

• [x] On Figure 1, move the legend to the right, use a stronger color for the data points that are not outliers, and make it black-and-white/color-blind friendly by using a different marker style for the outliers and changing line-styles (I'd remove the "True solution").

  The figure was replaced by a better example. We kept the "True solution" so readers can see how close to the solution is the parameters found by RAFF.

Suggestions: These can be disregarded it the authors prefer:

• [x] In the context of regression, I find that it's clearer to use x and y for the data and \beta or \theta for the parameters being optimized. Our traditional least squares problem min |Ax - b|^2 is usually written as min |Xβ - y|^2.

• [x] I find that entering the data separately for x and y is better, e.g.

  x = range(-1, 1, length=100)
  y = 2 .+ 3x + randn(100) * 0.1 
  model(β, x) = β[1] + β[2] * x[1]
  output = raff(model, x, y, 2) # Here

  The idea is great. It will be added to project in a future release. See #19 .

[fsobral]

Dear @abelsiqueira and @matbesancon , Thank you for your valuable comments and suggestions. We have added responses to most of your comments and questions. If there is no comment, it is because it has been fully accepted.

[matbesancon]

Thank you I should have time to review the changes between now and monday

[matbesancon]

Second review

Notation

Page 2: "we can sort the set in ascending order" The notation below is F_{i(theta)}(theta) while in the rest of the paper, it is F_{i}(theta), I didn't see why. For the notation again, it is counter-intuitive that the functions get re-ordered for every theta value. Because of this, it is hard to see how S_p(theta) is not differentiable (because p is still fixed, but the re-ordering of the indices switches which function is where).

Method

"RAFF.jl" solves this limitation by implementing a voting system. It is not clear from the end of last paragraph which limitation the authors are referring to.

How is the voting system brute-force?

"The solution which most occurs"

Solutions are not countable (two solutions are equal iff the functions yield the same image at any in any point in the domain, does that even happen with varying points?)

Phrasing

i.e., it solves the problem ... "i.e." is strangely spaced.

One of the most usual ways to solve the problem of least-squares

It would be good to emphasize "problem of non-linear least-squares", otherwise pseudo-matrix inversion would be preferred.

"master-slave implementation" --> prefer primary-worker / primary-secondary or other terms. master-slave now tends to be avoided in technical contexts.

[fsobral]

Second review

Notation

Page 2: "we can sort the set in ascending order" The notation below is F_{i(theta)}(theta) while in the rest of the paper, it is F_{i}(theta), I didn't see why. For the notation again, it is counter-intuitive that the functions get re-ordered for every theta value. Because of this, it is hard to see how S_p(theta) is not differentiable (because p is still fixed, but the re-ordering of the indices switches which function is where).

The LOVO function is not very easy to understand. One has to spend some time getting used to this terminology and notation. The easiest way of seeing the non differentiability of S_p(theta) is by observing that f_min(theta) = S_p(theta) for all theta, as discussed in the paragraph starting at line 15 on pg. 2. Also, f_min highlights the combinatorial spirit of the LOVO problem.

Regarding the notation, it is correct. When we write F_{i1(theta)}(theta) we mean that it is the smallest value in the set {F{i}(theta), i = 1, ..., m} for a given theta. F_{i_2(theta)}(theta) is the second smallest value in the set, and so on. The procedure for obtaining this is by evaluating all Fi at theta and ordering the set {F{i}(theta), i = 1, ..., m}. That's how the indices i_{k}(theta) are obtained. Since the values of F_i change for different values of theta, the order of the F_i's change too.

Therefore, we cannot change the notation, but we will add a small example to explain it better. We will ping you when it is finished.

Method

"RAFF.jl" solves this limitation by implementing a voting system. It is not clear from the end of last paragraph which limitation the authors are referring to.

How is the voting system brute-force?

"The solution which most occurs"

Solutions are not countable (two solutions are equal iff the functions yield the same image at any in any point in the domain, does that even happen with varying points?)

We have rephrase this paragraph to:

In this voting system, several LOVO subproblems are solved with different values for $p$, the number of possible trusted points. Each solution of a LOVO subproblem is associated to a vector parameter $\theta$. The vector parameters are compared against each other using the Euclidian distance, where small distances (using a threshold) are considered the same solution. The parameter $\theta^*$ which most occurs among them is declared as the solution.

Phrasing

i.e., it solves the problem ... "i.e." is strangely spaced.

One of the most usual ways to solve the problem of least-squares

It would be good to emphasize "problem of non-linear least-squares", otherwise pseudo-matrix inversion would be preferred.

"master-slave implementation" --> prefer primary-worker / primary-secondary or other terms. master-slave now tends to be avoided in technical contexts.

Done. Thanks for the observation

[abelsiqueira]

• At the end Functionality, mention that two points were manually set to be outliers.

I'm otherwise satisfied with the updates and the new version.

[fsobral]

* At the end Functionality, mention that two points were manually set to be outliers.

I'm otherwise satisfied with the updates and the new version.

You are correct. I have added the comment in the same phrase describing of the red triangles.

[fsobral]

@matbesancon We have added the simple example regarding the LOVO function. I hope it becomes clearer what we mean as its non differentiability.

[matbesancon]

Thanks, it looks good. Minor last things:

[...] therefore, can be run locally or in a cluster of computers

Shouldn't it be "on a cluster of computers"?

used by all experimental researchers who know a little about ...

This sentence looks a bit informal and can be interpreted in different ways: is it for experimental researchers only if they know a little, by experimental researchers even if they know little about, etc.

[fsobral]

Shouldn't it be "on a cluster of computers"?

Yes, you are right. Done.

used by all experimental researchers who know a little about ...

This sentence looks a bit informal and can be interpreted in different ways: is it for experimental researchers only if they know a little, by experimental researchers even if they know little about, etc.

The idea is that they do not need to know much about mathematical modeling, but definitely, they must know something, otherwise writing Julia functions defining models won't make any sense. I have rewritten this sentence to

This package is intended to be used by any experimental researcher with a little knowledge about mathematical modeling and fitting functions.

[matbesancon]

I'm ok with the changes thanks. Last tiny things in the reference, the letters are not properly capitalizd, you should use title = {{Title}} instead of title = {Title} everywhere

[evcastelani]

Hi @matbesancon! You're right. The title of the references had to be standardized. Thanks for the note. However, running locally by Pandoc, the use of {{tilte}} worked like a charm but in JOSS no. In this way, I made the manual correction of capital letters to avoid possible errors.

[matbesancon]

Hi, no it seems to have worked, the titles are properly capitalized. I think this was the last point on my side. @abelsiqueira should we close this issue?

[abelsiqueira]

Yes, I'm happy with the changes.

On Fri, May 24, 2019, 10:41 Mathieu Besançon notifications@github.com wrote:

Hi, no it seems to have worked, the titles are properly capitalized. I think this was the last point on my side. @abelsiqueira https://github.com/abelsiqueira should we close this issue?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/fsobral/RAFF.jl/issues/18?email_source=notifications&email_token=AAIE5UH2HQVM3Z46WX56RZ3PW7WBRA5CNFSM4HK6X3PKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWFMJPI#issuecomment-495633597, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIE5UB5WMKNJHDGJJ4LVRDPW7WBRANCNFSM4HK6X3PA .

[matbesancon]

Great, doing one last check on the last version of the software in an hour or so, then validating

[matbesancon]

sorry last small details in "installation and usage": "and can be directly installed from the Julia package repository" or "and can be directly installed from Julia's package repository" but the former might be preferred.

"All descriptions" or "The whole description"

[evcastelani]

@matbesancon you're right. These typos have been fixed.

[matbesancon]

ok great, the paper seems fine, watch out builds seem to all fail

[evcastelani]

@matbesancon Ok, thanks! Now we fixed the problem and both branches (master and joss) are passing the automated tests.

[matbesancon]

perfect, I think this is good on my side. @abelsiqueira do you still have something on the last version of the paper?

[abelsiqueira]

Reference Keles needs to be updated (DOI, pages and author name):

@article{Kele2018,
  doi = {10.15345/iojes.2018.03.013},
  url = {https://doi.org/10.15345/iojes.2018.03.013},
  year = {2018},
  publisher = {International Online Journal of Educational Sciences},
  volume = {10},
  number = {3},
  author = {Taliha Kele{\c{s}}},
  title = {Comparison of Classical Least Squares and Orthogonal Regression in Measurement Error Models},
  journal = {International Online Journal of Educational Sciences}
}

Found the DOI on the paper and the bibtex using https://www.doi2bib.org/bib/10.15345/iojes.2018.03.013.

That's it for me.

[evcastelani]

@abelsiqueira you're right! We fixed DOI,name and pages as you suggest.

[evcastelani]

@matbesancon and @abelsiqueira Can I close this issue?

[matbesancon]

If you re-built the paper with corrections from @abelsiqueira https://github.com/abelsiqueira it's up to him to close, it's good on my side

On Wed, May 29, 2019, 19:13 evcastelani notifications@github.com wrote:

@matbesancon https://github.com/matbesancon and @abelsiqueira https://github.com/abelsiqueira Can I close this issue?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/fsobral/RAFF.jl/issues/18?email_source=notifications&email_token=AB2FDMQPFFLNVKSAJC4BS33PX22UJA5CNFSM4HK6X3PKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWQAVOA#issuecomment-497027768, or mute the thread https://github.com/notifications/unsubscribe-auth/AB2FDMX774HP3BKZ74EEVITPX22UJANCNFSM4HK6X3PA .

[evcastelani]

@matbesancon Ok, you're right.

[abelsiqueira]

Yes, close it, thanks for the corrections.