-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[JOSS] Paper review 1 #18
Comments
|
@evcastelani thanks for the answer list. For the algebraic part, putting this explanation and/or references in the paper would be useful |
ping me when the editions are done |
ReviewThe 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.
Suggestions: These can be disregarded it the authors prefer:
|
@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.
Suggestions: These can be disregarded it the authors prefer:
|
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. |
Thank you I should have time to review the changes between now and monday |
Second reviewNotationPage 2: "we can sort the set in ascending order" 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?
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
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. |
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_{i_1(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 F_i 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.
We have rephrase this paragraph to: In this voting system, several LOVO subproblems are solved
Done. Thanks for the observation |
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. |
@matbesancon We have added the simple example regarding the LOVO function. I hope it becomes clearer what we mean as its non differentiability. |
Thanks, it looks good. Minor last things:
Shouldn't it be "on a cluster of computers"?
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. |
Yes, you are right. Done.
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 |
I'm ok with the changes thanks. Last tiny things in the reference, the letters are not properly capitalizd, you should use |
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 |
Hi, no it seems to have worked, the titles are properly capitalized. I think this was the last point on my side. |
Yes, I'm happy with the changes.
…On Fri, May 24, 2019, 10:41 Mathieu Besançon ***@***.***> 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
<#18?email_source=notifications&email_token=AAIE5UH2HQVM3Z46WX56RZ3PW7WBRA5CNFSM4HK6X3PKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWFMJPI#issuecomment-495633597>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAIE5UB5WMKNJHDGJJ4LVRDPW7WBRANCNFSM4HK6X3PA>
.
|
Great, doing one last check on the last version of the software in an hour or so, then validating |
sorry last small details in "installation and usage": "All descriptions" or "The whole description" |
@matbesancon you're right. These typos have been fixed. |
ok great, the paper seems fine, watch out builds seem to all fail |
@matbesancon Ok, thanks! Now we fixed the problem and both branches (master and joss) are passing the automated tests. |
perfect, I think this is good on my side. @abelsiqueira do you still have something on the last version of the paper? |
Reference Keles needs to be updated (DOI, pages and author name):
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. |
@abelsiqueira you're right! We fixed DOI,name and pages as you suggest. |
@matbesancon and @abelsiqueira Can I close this issue? |
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 ***@***.***> 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
<#18?email_source=notifications&email_token=AB2FDMQPFFLNVKSAJC4BS33PX22UJA5CNFSM4HK6X3PKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWQAVOA#issuecomment-497027768>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB2FDMX774HP3BKZ74EEVITPX22UJANCNFSM4HK6X3PA>
.
|
@matbesancon Ok, you're right. |
Yes, close it, thanks for the corrections. |
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 itSecond 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 anyReal
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 openjournals/joss-reviews#1385
The text was updated successfully, but these errors were encountered: