sympy / sympy-paper

Repo for the paper "SymPy: symbolic computing in python"
https://peerj.com/articles/cs-103/
Other
47 stars 37 forks source link

Article Approval #217

Open certik opened 7 years ago

certik commented 7 years ago

Dear co-authors (@asmeurer, @smichr, @mattpap, @certik, @skirpichev, @mrocklin, @aktech, @scolobb, @moorepants, @leosartaj, @thilinarmtb, @flacjacket, @ellisonbg, @rpmuller, @Upabjojr, @hargup, @shivamvats, @fredrik-johansson, @fabianp, @mattcurry, @aterrel, @rouckas, @ashutoshsaboo, @isuruf, @Sumith1896, @rc, @scopatz), in #195 all of us approved the 4 ICMJE authorship criteria. Since then, we made several revisions on the paper, based on two rounds of peer review, so we are asking each of you to approve the latest version, since the paper has changed substantially since the last time it got approved by all of us. The current status is that the second round of reviews were addressed, and we are now waiting for PeerJ to send a list of technical changes (e.g. tweaks to images, titles and legends, declarations etc.), those are minor. After that, we'll submit. There is high chance it will get accepted, but if not, we'll do another round.

Post "I approve" if you approve the current version (see the next section for how to see the latest version).

Latest Version

Commit: c79113cffdf5b7b527e0143920e9eabed53dc8a5 (part of #216) Generated pdf documents for this commit are here: https://github.com/isuruf-bot/sympy-paper/commit/b3b611e473225d6144876c6556e3b64a4fa36347 Specifically: paper-216.pdf paper-216-supplement.pdf

In addition, the following metadata was submitted to PeerJ, and will appear in the final article as published by PeerJ:

Metadata

Please provide your Competing Interest statement here using complete sentences. This may include financial, non-financial, professional or personal relationships, including serving as an Academic Editor for PeerJ. If there are no competing interests then you must explicitly state this fact. This text will be published alongside your accepted manuscript.

Christopher P Smith is an employee of Polar Semiconductor, Inc., Bloomington, Minnesota, United States; Mateusz Paprocki and Matthew Rocklin are employees of Continuum Analytics, Inc., Austin, Texas, United States; Andy R Terrel is an employee of Fashion Metric, Inc, Austin, Texas, United States.

Funding Statement: Please provide a statement (using complete sentences) describing the funding sources for your work. Name the funding source/grant agency and include any grant or identification numbers. This statement will be published alongside the final manuscript.

Google Summer of Code has provided financial support to students who contributed to SymPy. The author of this paper Ondřej Čertík was supported by the Los Alamos National Laboratory. The Los Alamos National Laboratory is operated by Los Alamos National Security, LLC, for the National Nuclear Security Administration of the U.S. Department of Energy under Contract No. DE-AC52-06NA25396. The author of this paper Richard P. Muller was supported by Sandia National Laboratories. Sandia is a multiprogram laboratory operated by Sandia Corporation, a Lockheed Martin Company, for the United States Department of Energy's National Nuclear Security Administration under Contract DE-AC04-94AL85000. The author of this paper Francesco Bonazzi was supported by Deutsche Forschungsgemeinschaft (DFG) via the International Research Training Group 1524 “Self- Assembled Soft Matter Nano-Structures at Interfaces.”

These co-authors approved the final version:

asmeurer commented 7 years ago

I approve.

certik commented 7 years ago

I approve.

flacjacket commented 7 years ago

I approve

rc commented 7 years ago

I approve.

aktech commented 7 years ago

I approve.

scopatz commented 7 years ago

I approved in the other issue

Sumith1896 commented 7 years ago

I approve.

rouckas commented 7 years ago

I approve

mattcurry commented 7 years ago

I approve.

fredrik-johansson commented 7 years ago

I approve.

certik commented 7 years ago

@skirpichev replied:

I disagree on the first point of the editor.

I doubt we need this footnote: both original sentences are correct if standard notion of sqrt(x) was used. I believe that it's clear that we don't use multivalued functions in this context and thus - any changes here are useless and redundant. At most - we could mention that our sqrt is the principal square root.

However, if you are sure that mentioned in the footnote identity is correct - I'm ok with this footnote as is. Please note, that it's clearly not so in the SymPy (for t=0):

arg(0) nan

Everything else looks acceptable for me and I approve all with the minor note above.

Thank you.

asmeurer commented 7 years ago

I agree for t=0 the formula only works if you take a limit. I believe it works because arg(t) is defined away from 0 and (-1)^... has absolute value 1, so the limit (-1)^... * t as t -> 0 goes to 0. Couldn't one just as well write it as a piecewise sqrt(t**2) = {t, if <condition on arg(t)>, -t, otherwise? But this is already way more technical than necessary. The whole point is that sqrt(t**2) isn't equal to t for all complex numbers t. That's the only thing you need to know. What do you think Ondrej?

certik commented 7 years ago

Well, the limit t->0 seems to exist and be 0, but I didn't realize the complication with t=0 before. I don't think it's worth mentioning all these technicalities, so I propose we remove the footnote and rework the sentences to stress that sqrt(t^2) is not equal to t for all complex t and mention that our sqrt() is defined on the principal branch, as usual. I've done that in c79113cffdf5b7b527e0143920e9eabed53dc8a5...f2cb16e7c1aaaa075f2c8c11d43d53a594b2b303, so all should be good now.

hargup commented 7 years ago

I approve.

certik commented 7 years ago

@skirpichev approves f2cb16e7c1aaaa075f2c8c11d43d53a594b2b303.

Upabjojr commented 7 years ago

In the PDF, line 329:

Internally these matrices store the elements as Lists of Lists (LIL) [27], meaning the matrix is stored as a list of lists of entries (effectively, the input format used to create the matrix A above), making it a dense representation.

In [1]: a = Matrix([[x,y],[z,w]])

In [2]: a
Out[2]: 
⎡x  y⎤
⎢    ⎥
⎣z  w⎦

In [4]: a._mat
Out[4]: [x, y, z, w]

Are you sure this is a list of lists? It looks as a one-dimensional list to me.

isuruf commented 7 years ago

It is a one-dimensional list, not a list of lists.

fredrik-johansson commented 7 years ago

That part did strike me as being a bit redundant, and the distinction between a list and a list of lists is not really that important anyway. The paragraph could be shortened to something like:

"Internally these matrices use a dense format, storing all entries in a list. For sparse matrices, the SparseMatrix class can be used. SparseMatrix stores only the nonzero entries, using a dict to map (row, column) pairs to elements."

asmeurer commented 7 years ago

c.f. reviewer 3 point 23 in the rebuttal.pdf (the first rebuttal) on the LIL/DOK thing.

mrocklin commented 7 years ago

Seems fine to me

mattpap commented 7 years ago

I approve.

isuruf commented 7 years ago

I approve

Upabjojr commented 7 years ago

I approve.

shivamvats commented 7 years ago

I approve.

leosartaj commented 7 years ago

I approve.

On 21 October 2016 at 10:51, Harsh Gupta notifications@github.com wrote:

I approve.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sympy/sympy-paper/issues/217#issuecomment-255296098, or mute the thread https://github.com/notifications/unsubscribe-auth/AF7g-MzBoWOzquFecI-zPCbrSsomISkFks5q2EvagaJpZM4KcdsS .

Regards Sartaj Singh

Mathematics and Computing, Indian Institute of Technology, Varanasi - 221 005 INDIA

E-mail: singhsartaj94@gmail.com, sartaj.singh.apm13@itbhu.ac.in sartaj.singh.apm13@itbhu.ac.in

thilinarmtb commented 7 years ago

I approve.

Regards, Thilina

On Fri, Oct 21, 2016 at 6:17 AM, Fredrik Johansson <notifications@github.com

wrote:

That part did strike me as being a bit redundant, and the distinction between a list and a list of lists is not really that important anyway. The paragraph could be shortened to something like:

"Internally these matrices use a dense format, storing all entries in a list. For sparse matrices, the SparseMatrix class can be used. SparseMatrix stores only the nonzero entries, using a dict to map (row, column) pairs to elements."

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sympy/sympy-paper/issues/217#issuecomment-255356206, or mute the thread https://github.com/notifications/unsubscribe-auth/ABSM3Gywa2ZecUtDvEKzfNtBUy8MX2_wks5q2J9NgaJpZM4KcdsS .

ashutoshsaboo commented 7 years ago

I approve.

On 21 Oct 2016 3:31 am, "Ondřej Čertík" notifications@github.com wrote:

Well, the limit t->0 seems to exist and be 0, but I didn't realize the complication with t=0 before. I don't think it's worth mentioning all these technicalities, so I propose we remove the footnote and rework the sentences to stress that sqrt(t^2) is not equal to t for all complex t and mention that our sqrt() is defined on the principal branch, as usual.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sympy/sympy-paper/issues/217#issuecomment-255240612, or mute the thread https://github.com/notifications/unsubscribe-auth/AJcTkdgA_mPB774dYAzSegsBhqLvtefCks5q1-SrgaJpZM4KcdsS .

rpmuller commented 7 years ago

I approve.

On Thu, Oct 20, 2016 at 2:39 PM Ondřej Čertík notifications@github.com wrote:

Dear co-authors (@asmeurer https://github.com/asmeurer, @smichr https://github.com/smichr, @mattpap https://github.com/mattpap, @certik https://github.com/certik, @skirpichev https://github.com/skirpichev, @mrocklin https://github.com/mrocklin, @aktech https://github.com/aktech, @scolobb https://github.com/scolobb, @moorepants https://github.com/moorepants, @leosartaj https://github.com/leosartaj, @thilinarmtb https://github.com/thilinarmtb, @flacjacket https://github.com/flacjacket, @ellisonbg https://github.com/ellisonbg, @rpmuller https://github.com/rpmuller, @Upabjojr https://github.com/Upabjojr, @hargup https://github.com/hargup, @shivamvats https://github.com/shivamvats, @fredrik-johansson https://github.com/fredrik-johansson, @fabianp https://github.com/fabianp, @mattcurry https://github.com/mattcurry, @aterrel https://github.com/aterrel, @rouckas https://github.com/rouckas, @ashutoshsaboo https://github.com/ashutoshsaboo, @isuruf https://github.com/isuruf, @Sumith1896 https://github.com/Sumith1896, @rc https://github.com/rc, @scopatz https://github.com/scopatz), in #195 https://github.com/sympy/sympy-paper/issues/195 all of us approved the 4 ICMJE authorship criteria http://www.icmje.org/recommendations/browse/roles-and-responsibilities/defining-the-role-of-authors-and-contributors.html#two. Since then, we made several revisions on the paper, based on two rounds of peer review, so we are asking each of you to approve the latest version, since the paper has changed substantially since the last time it got approved by all of us. The current status is that the second round of reviews were addressed, and we are now waiting for PeerJ to send a list of technical changes (e.g. tweaks to images, titles and legends, declarations etc.), those are minor. After that, we'll submit. There is high chance it will get accepted, but if not, we'll do another round.

Post "I approve" if you approve the current version (see the next section for how to see the final version). Latest Version

Commit: c79113c https://github.com/sympy/sympy-paper/commit/c79113cffdf5b7b527e0143920e9eabed53dc8a5 (part of #216 https://github.com/sympy/sympy-paper/pull/216) Generated pdf documents for this commit are here: isuruf-bot@b3b611e https://github.com/isuruf-bot/sympy-paper/commit/b3b611e473225d6144876c6556e3b64a4fa36347 Specifically: paper-216.pdf https://github.com/isuruf-bot/sympy-paper/raw/b3b611e473225d6144876c6556e3b64a4fa36347/paper-216.pdf paper-216-supplement.pdf https://github.com/isuruf-bot/sympy-paper/raw/b3b611e473225d6144876c6556e3b64a4fa36347/paper-216-supplement.pdf

In addition, the following metadata was submitted to PeerJ, and will appear in the final article as published by PeerJ: Metadata

Please provide your Competing Interest statement here using complete sentences. This may include financial, non-financial, professional or personal relationships, including serving as an Academic Editor for PeerJ. If there are no competing interests then you must explicitly state this fact. This text will be published alongside your accepted manuscript.

Christopher P Smith is an employee of Polar Semiconductor, Inc., Bloomington, Minnesota, United States; Mateusz Paprocki and Matthew Rocklin are employees of Continuum Analytics, Inc., Austin, Texas, United States; Andy R Terrel is an employee of Fashion Metric, Inc, Austin, Texas, United States.

Funding Statement: Please provide a statement (using complete sentences) describing the funding sources for your work. Name the funding source/grant agency and include any grant or identification numbers. This statement will be published alongside the final manuscript.

Google Summer of Code has provided financial support to students who contributed to SymPy. The author of this paper Ondřej Čertík was supported by the Los Alamos National Laboratory. The Los Alamos National Laboratory is operated by Los Alamos National Security, LLC, for the National Nuclear Security Administration of the U.S. Department of Energy under Contract No. DE-AC52-06NA25396. The author of this paper Richard P. Muller was supported by Sandia National Laboratories. Sandia is a multiprogram laboratory operated by Sandia Corporation, a Lockheed Martin Company, for the United States Department of Energy's National Nuclear Security Administration under Contract DE-AC04-94AL85000. The author of this paper Francesco Bonazzi was supported by Deutsche Forschungsgemeinschaft (DFG) via the International Research Training Group 1524 “Self- Assembled Soft Matter Nano-Structures at Interfaces.” These co-authors approved the final version:

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sympy/sympy-paper/issues/217, or mute the thread https://github.com/notifications/unsubscribe-auth/AAchshqMl67z5AZvgQqZnn_rFOMP5m4rks5q17VngaJpZM4KcdsS .

aterrel commented 7 years ago

I approve.

asmeurer commented 7 years ago

@smichr @scolobb @moorepants @ellisonbg @fabianp please sign off.