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

make sure _onenorm_matrix_power_nnm actually returns a float #8302

Merged
merged 1 commit into from Jan 19, 2018

Conversation

nschloe
Copy link
Contributor

@nschloe nschloe commented Jan 16, 2018

The function _onenorm_matrix_power_nnm() computes the 1-norm of a non-negative integer power of a non-negative matrix and

Returns
    -------
    out : float
        The 1-norm of the matrix power p of A.

However, it really does return numpy.array([alpha]) with alpha being the 1-norm. This PR fixes things by actually returning a float.

Before:

print(_onenorm_matrix_power_nnm(A, p))   # array([1.31])

After:

print(_onenorm_matrix_power_nnm(A, p))   # 1.31

The change is probably too small to warrant an explicit test.

Used to be a numpy.array of size 1 up until now.
@rgommers rgommers added scipy.sparse.linalg maintenance Items related to regular maintenance tasks labels Jan 19, 2018
@rgommers
Copy link
Member

The change is probably too small to warrant an explicit test.

It would be good to at least show the old and new behavior in the PR description. From a quick look it looks like this affects the behavior of expm. Not sure about the other variants of expm?

@nschloe
Copy link
Contributor Author

nschloe commented Jan 19, 2018

@rgommers I've extended the description to highlight the difference.

@pv
Copy link
Member

pv commented Jan 19, 2018 via email

@rgommers
Copy link
Member

@nschloe are you using that private function directly? The only public one that uses it seems to be expm, however that returns a matrix:

In [19]: A
Out[19]: 
<1x1 sparse matrix of type '<class 'numpy.int64'>'
	with 1 stored elements in Compressed Sparse Column format>

In [20]: A.todense()
Out[20]: matrix([[1]], dtype=int64)

In [21]: sl.expm(A).todense()
Out[21]: matrix([[2.71828183]])

@nschloe
Copy link
Contributor Author

nschloe commented Jan 19, 2018

@rgommers Not really, it's just something I noticed when I fiddled with numpy for other reasons. It seemed easy enough to fix it, so that's what I did.

@rgommers
Copy link
Member

Okay, thanks for clarifying. Since it doesn't change any user-visible behavior and np.max was intended, let's just merge this.

Thanks @nschloe, @pv

@rgommers rgommers merged commit 7c3c968 into scipy:master Jan 19, 2018
@rgommers rgommers added this to the 1.1.0 milestone Jan 19, 2018
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.linalg
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants