Review process for contributions

[Jernej Azarija]

Hello!

I have noticed (at least in the fields to which I made some small contributions) that the number of reviewers is arbitrary. Sometimes there is only one reviewer sometimes two, three..

I cannot speak for others, but I wouldn't want to be the only reviewer of a patch since I am quick to overlook things and make stupid assumptions. 

Hence I am wondering, if you ever considered putting some norm on the number of reviewers for new patches? Like 3 reviewers before the patch is taken into consideration? 

This appears to be the standard for research publications and since one would want to use Sage as part of their research project it seems like a sane option as well.

As is, I am not 100% comfortable completely relying on Sage for results related to mathematical research.

Best,

Jernej

[kcrisman]

On Thursday, February 28, 2013 11:26:20 AM UTC-5, Jernej Azarija wrote:

Hello!

I have noticed (at least in the fields to which I made some small contributions) that the number of reviewers is arbitrary. Sometimes there is only one reviewer sometimes two, three..

I cannot speak for others, but I wouldn't want to be the only reviewer of a patch since I am quick to overlook things and make stupid assumptions. 

That's okay - you can always say that you contributed review to some portion of the patch, and leave it to another to give final positive review.

 

This appears to be the standard for research publications and since one would want to use Sage as part of their research project it seems like a sane option as well.

I'd be interested in other comments on this, but I think the balance we have works pretty well; we tend to have a significantly higher level of review than many other open source efforts, I think, since there is no pool of "committers".

 

As is, I am not 100% comfortable completely relying on Sage for results related to mathematical research.

You should never trust a computer 100%, and in fact many people who use Sage confirm their computations with other software (and vice versa).  That said, I would trust a program that didn't open up its source code even less for this...

[Nathann Cohen]

Helloooooooooooooooooooooooooo !!!

I have noticed (at least in the fields to which I made some small contributions) that the number of reviewers is arbitrary. Sometimes there is only one reviewer sometimes two, three..

I cannot speak for others, but I wouldn't want to be the only reviewer of a patch since I am quick to overlook things and make stupid assumptions. 

Well. There is a review process because the author of a patch is assumed to be quick to overlook things and make stupid assumptions. 

Hence I am wondering, if you ever considered putting some norm on the number of reviewers for new patches? Like 3 reviewers before the patch is taken into consideration? 

Noooooooooooooooooooooooooooooooooooooooooooooooooo !! No more ruuuuuuuuuuules ! Pleaaaaaaaaaaaaaaaase ! :-D

Anyway, the first step if you declare that you need 3 reviews before getting something included inside of Sage is that it will be much, much more difficult to get anything inside of Sage, and some ticket (nooooooo I'm not thinking of combinat :-D) have already been waiting for a review for a looooook time.

Of course, requiring ONE review instead of accepting the code immediately has the same effect. And the current way is not that bad, actually !

The point is that I would be totally amazed if #12224 were to (ever) be reviewed. Do you think that it could be reviewed twice ? :-P

This appears to be the standard for research publications

Ahahaah. Yeah, but research publications (at least in my field) are read by three reviewers and buried forever, never to be read again. Sage's code is doctested, and used.

 

and since one would want to use Sage as part of their research project it seems like a sane option as well.

As is, I am not 100% comfortable completely relying on Sage for results related to mathematical research.

I don't. You run code, the code fails, it's a bug from Sage's code or a bug from your code. The point is that I don't even hope for Sage to ever be bug-free. It's a software, not the Bible. Which is never wrong, of course.

I think that the problem is that people assume that research papers are correct. There's one trick I love to tell : have you ever seen a published paper whose title is "Erratum : the proof given in [...] is wrong, here is how it can be fixed" ? I assume you have. Now, have you ever seen a published paper whose title is "Erratum : the proof given in [...] is wrong, and we have no idea how to fix it" ? I never did.

There are wrong results, out there. During my studies, the first 3 research papers I read all contained an error which destroyed their main result. Bad luck perhaps, but still.

The proofs of many research papers (in my area of research) are soooooo complicated, sooooooo uninteresting, and sooooo built on the model "Case 1 / Subcase 3 / Subsubcase 35" that any sane person would be fighting against sleep after page two.

I trust much, much more things like Sage (whose behaviour can be rather quickly understood by reading some code) than some complicated paper based on other results based on other results, with long and unclear proofs based on case studies. Nobody can check that. 

I think that the best thing to do is actually to stop believing in research papers for any serious research work.

When the hell did we stop accepting as True only the theorems whose proof we clearly understand, to settle for the weaker version of truth that is "it has been published in a paper somewhere" ?

It conviced 3 random guys I have never met. Of curse it is true.

As usual, my contribution to this thread is pointless. But people take so many things for granted that I sometime fight against their right to doubt anything at all, on principle.

Have fuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuunnn !!

Nathann

[luisfe]

On 28 feb, 17:26, Jernej Azarija <azi.std...@gmail.com> wrote:
> Hello!
>
> I have noticed (at least in the fields to which I made some small
> contributions) that the number of reviewers is arbitrary. Sometimes there
> is only one reviewer sometimes two, three..
>
> I cannot speak for others, but I wouldn't want to be the only reviewer of a
> patch since I am quick to overlook things and make stupid assumptions.

Then review a patch and, before giving positive review, take a mental
break and review it again :)

> Hence I am wondering, if you ever considered putting some norm on the
> number of reviewers for new patches? Like 3 reviewers before the patch is
> taken into consideration?

I think it is ok as it is. If you require say three reviewers then
things would slow down too much. If there is something tricky you can
always say that you have looked here and there but that this or that
part should be reviewed by another person.

> This appears to be the standard for research publications and since one
> would want to use Sage as part of their research project it seems like a
> sane option as well.
>
> As is, I am not 100% comfortable completely relying on Sage for results
> related to mathematical research.

One may also think that with the at least one reviewer rule the sage
library may have more eyes looking at its code than some of the
underlying software...

> Best,
>
> Jernej

[luisfe]

The point is that I would be totally amazed if #12224 were to (ever) be reviewed. Do you think that it could be reviewed twice ? :-P

Do not despair, my pet bug #10255 has the patch ready since two years ago... ugh, that hurts. Anyone willing for reviewing it? :D 

[Nathann Cohen]

> Do not despair, my pet bug #10255 has the patch ready since two years ago... ugh, that hurts. Anyone willing for reviewing it? :D

Aahaah. Come on, your patch seems realistically reviewable ! 12224 weighs 1.3 MB :-D

Nathann

[Robert Bradshaw]

On Thu, Feb 28, 2013 at 9:19 AM, luisfe <lfta...@yahoo.es> wrote:
> On 28 feb, 17:26, Jernej Azarija <azi.std...@gmail.com> wrote:
>> Hello!
>>
>> I have noticed (at least in the fields to which I made some small
>> contributions) that the number of reviewers is arbitrary. Sometimes there
>> is only one reviewer sometimes two, three..
>>
>> I cannot speak for others, but I wouldn't want to be the only reviewer of a
>> patch since I am quick to overlook things and make stupid assumptions.
>
> Then review a patch and, before giving positive review, take a mental
> break and review it again :)

A single reviewer adds a lot of value, but the marginal benefit per
reviewer goes down quickly while the marginal cost goes up. That being
said, if you don't feel confortable giving something a positive
review, just leave some comments (perhaps even setting it to needs
work) and move on. Some code takes more than one person to look at (or
even write), but I don't think that should become the norm. It's not
like a paper where the alternative to acceptance is (expensive)
rejection.

>> Hence I am wondering, if you ever considered putting some norm on the
>> number of reviewers for new patches? Like 3 reviewers before the patch is
>> taken into consideration?
>
> I think it is ok as it is. If you require say three reviewers then
> things would slow down too much. If there is something tricky you can
> always say that you have looked here and there but that this or that
> part should be reviewed by another person.
>
>> This appears to be the standard for research publications and since one
>> would want to use Sage as part of their research project it seems like a
>> sane option as well.
>>
>> As is, I am not 100% comfortable completely relying on Sage for results
>> related to mathematical research.

Does any software you want to use for research consistently 3
reviewers beyond the initial author? Also, at some point it's much
easier to find bugs when software is actually used. Also, in my mind
*quality* testing acts as a second data point (that, better than
anything research papers have, keeps reviewing and reviewing and
reviewing as the code around/underneath it changes).

- Robert

[Nathann Cohen]

A single reviewer adds a lot of value, but the marginal benefit per
reviewer goes down quickly while the marginal cost goes up. That being
said, if you don't feel confortable giving something a positive
review, just leave some comments (perhaps even setting it to needs
work) and move on.

Oh. And if you remember, please say so explicitly on the ticket. Something like "I don't feel comfortable, so I will let somebody else review it". Otherwise people (err.. I) assume that you are still working on the review and will not do it myself if I find this ticket on the trac server :-)

Nathann

[kcrisman]

Good comments, Nathann, though some papers are read more than others :)

Ahahaah. Yeah, but research publications (at least in my field) are read by three reviewers and buried forever, never to be read again. Sage's code is doctested, and used.

THREE reviewers?  Most of mine have had ONE, some TWO.  I've only seen three in pedagogical journals thus far.  You must have a very fruitful field with lots of people who don't turn down the opportunity to referee!

[Nathann Cohen]

Yoooooooooooo !

> Good comments, Nathann, though some papers are read more than others :)

Yepyep, definitely. Some smart ones. And it's actually because they are read many times by guys who actually want too understand that you believe them rather easily. A bit like we can trust some Sage code because it is used by many persons for different things :-)

> THREE reviewers?  Most of mine have had ONE, some TWO.  I've only seen
> three in pedagogical journals thus far.  You must have a very fruitful field
> with lots of people who don't turn down the opportunity to referee!

Ahahahaha. Well, I would say the same in my field, too. And. Well. Here's a review I received some time ago :

---
Reviewer #2: An interesting paper. I have only some minor remarks.

- page 5, line 17:  we but have ...  ???
[...]
---

Makes me think I should have put more erroneous claims in that paper. Unfortunately, reviewer #1 actually read it :-)

Nathn

[Travis Scrimshaw]

Also either put a ... after your name in the reviews or not at all, otherwise I (and I'm sure others) would think you are the only person needed for/doing the review.

Best,

Travis

[Simon King]

On 2013-02-28, kcrisman <kcri...@gmail.com> wrote:
> ------=_Part_444_3892362.1362074924783
> Content-Type: text/plain; charset=ISO-8859-1

>
> Good comments, Nathann, though some papers are read more than others :)
>
> Ahahaah. Yeah, but research publications (at least in my field) are read by
>> three reviewers and buried forever, never to be read again. Sage's code is
>> doctested, and used.
>>
>
> THREE reviewers? Most of mine have had ONE, some TWO.

Same here.

And concerning "three reviewers for each patch on Sage trac": Don't
forget that there are more or less trivial patches.
* There are patches fixing a typo in some doc string---I think
this hardly justifies having more than *zero* referees!
* There are patches which have a rather local effect, because they
change a method that is used in some specialised computations, but
not in other modules of Sage. Here, one specialist in the concerned
field of maths is enough for a review.
* There are patches which improve existing functionality by better
coding. Here, someone with a background in software engineering is
enough for a review.
* There are patches changing, say, the way how coercion maps are found
or how parent and element classes are built. This has an influence in
almost all components of Sage, and typically is quite complicated
stuff. And here, it naturally occurs that a group of people gathers on
the ticket, each one contributing a patch and reviewing the patches of
the other people.

So, I think there shouldn't be a rule for the number of reviewers,
apart from it being a positive number.

Cheers,
Simon