Skip to content
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

ENH: Change sparse .sum and .mean dtype casting to match NumPy. #5907

Merged
merged 4 commits into from
Mar 6, 2016

Conversation

Tiepies
Copy link
Contributor

@Tiepies Tiepies commented Feb 29, 2016

Change the sparse .sum and .mean methods to behave like their NumPy
matrix counterparts in terms of dtype casting, and update the
corresponding tests in scipy/sparse/test_base.py. This should resolve #2534.

Change the sparse `.sum` and `.mean` methods to behave like their NumPy
matrix counterparts in terms of dtype casting, and update the
corresponding tests in scipy/sparse/test_base.py. This should solve
issue scipy#2534.
rtol = 1e-04
else:
rtol = 1e-07
assert_allclose(NC.A, M.A, rtol=rtol)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I included it because the assert_allclose test failed at the default rtol for _NonCanonicalMixin subclasses initialized with np.random.rand(40, 40) and sparse.rand(40, 40, density=1e-2).A (used in test_sum) for the dtypes np.float32 and np.complex64.

Comparisons between the results of the sparse and dense matrix `.sum`
methods in `sparse.test_base.test_sum` fail for matrices containing
single-precision floats (`np.float32` and `np.complex64`) when the
sparse and dense sums are calculated differently and the resulting sum
values have too many digits to be considered equal in single-precision
float format with the default value of the `decimal` parameter of
`assert_array_almost_equal`. This is fixed by using smaller test
matrices, so that the number of digits of their sums remains small
enough in all cases for the values to be considered equal at the
default value of the `decimal` parameter.
@codecov-io
Copy link

@@            master   #5907   diff @@
======================================
  Files          238     238       
  Stmts        43592   43598     +6
  Branches      8179    8180     +1
  Methods          0       0       
======================================
+ Hit          34020   34026     +6
  Partial       2602    2602       
  Missed        6970    6970       

Review entire Coverage Diff as of e889ff8

Powered by Codecov. Updated on successful CI builds.

perimosocordiae added a commit that referenced this pull request Mar 6, 2016
ENH: Change sparse `.sum` and `.mean` dtype casting to match NumPy.
@perimosocordiae perimosocordiae merged commit 65df486 into scipy:master Mar 6, 2016
@perimosocordiae
Copy link
Member

Merged. Thanks, @Tiepies!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Items related to regular maintenance tasks scipy.sparse
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The sparse .sum() method with uint8 dtype does not act like the corresponding numpy method.
5 participants