search

mdhaber / scipy

Scipy library main repository
http://scipy.org/scipylib/
BSD 3-Clause "New" or "Revised" License
1 stars 5 forks source link

Add Performance section #83

Closed mdhaber closed 2 years ago

mdhaber commented 2 years ago

Reference issue

Closes gh-82

What does this implement/fix?

This adds a section about the performance of scipy.stats.studentized_range.

Aditional information

I initially decided against a separate section for this because performance claims often need to be qualified with lots of details. I have omitted these here, but I believe the statements are representative of what one would conclude upon more careful review of the referenced paper.

mdhaber commented 2 years ago

@acolum - it sounds like this addresses the concerns in gh-82, but i also wanted to check with you - do you agree with these changes?

acolum commented 2 years ago

@mdhaber Thank you for letting me know about these changes. I agree with the changes, but I also spotted some small issues:

  1. On line 83, "cumulative density function" should be changed to "cumulative distribution function."
  2. On line 84: "six order of magnitude" -> "six orders of magnitude"
  3. I would recommend switching the order of the "Performance" and "Statement of need" sections to make the latter first, but still have both immediately follow the "Summary" section.
mdhaber commented 2 years ago

@acolum Thank you for catching the mistakes. (I can't believe I let "cumulative density function" sneak in again!)

Is it OK to keep the scope of this PR to adding the performance section? I'm hesitant to also change the order of the sections here, since the purpose is to address gh-82. I will open another PR with that suggestion.

Oops... I did intend to fix those mistakes here. Done in cd51a54eac025474df5259a73561a0b74f541abd.

mcavs commented 2 years ago

I also agree on these changes @mdhaber.