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

minor change: added norm examples to linalg.rst docs #365

Merged
merged 4 commits into from Feb 10, 2013

Conversation

Projects
None yet
3 participants
@cirosantilli
Contributor

cirosantilli commented Nov 27, 2012

norm had no examples, it is easier to understand with a few simple ones, will save people 1min of their lives

>>> A=mat('[1, 2; 3, 4]')
>>> A
matrix([[1, 2],
[3, 4]])

This comment has been minimized.

@rgommers

rgommers Nov 27, 2012

Member

Could you change this to:

>>> A = np.array([[1, 2], [3, 4]])
>>> A
array([[1, 2],
       [3, 4]])

Use of np.matrix should be discouraged.

[3, 4]])
>>> linalg.norm(A)
5.4772255750516612
>>> linalg.norm(A,'fro') #'fro' is default

This comment has been minimized.

@rgommers

rgommers Nov 27, 2012

Member

Perhaps more explicit: # Frobenius norm (the default).

5.4772255750516612
>>> linalg.norm(A,'fro') #'fro' is default
5.4772255750516612
>>> linalg.norm(A,1)

This comment has been minimized.

@rgommers

rgommers Nov 27, 2012

Member

Also add a comment here? ``# L1-norm

@rgommers

This comment has been minimized.

Member

rgommers commented Nov 27, 2012

Thanks for this, adding an example looks useful.

@cirosantilli

This comment has been minimized.

Contributor

cirosantilli commented Nov 28, 2012

agree on all points, one can never be explicit enough.

I didn't know that matrix was discouraged, there are other parts on this doc file that use it, and I was copying what I saw. should I modify those parts to use np.array instead, and put a note saying that matrix is discouraged? if yes, I need to know two things to do that myself (noob here):

  1. why is it discouraged
  2. what is the preferred way of importing inv from linalg?
  • import numpy.linalg and doing linalg.inv which is quite verbose,
  • from numpy.linalg import inv
  • something else
@rgommers

This comment has been minimized.

Member

rgommers commented Nov 28, 2012

If you could change the rest of that file, that would be great. It has been discussed several times on the mailing list whether or not to remove matrix. The reason is that there's some inconsistencies between matrix and ndarray, and there's really nothing it adds besides notational convenience. In the end it was kept because a few people found it useful, but it's advised not to use it (except perhaps for teaching linear algebra).

The standard import convention is import numpy as np; that line doesn't have to be added to all examples. So you'd use inv like: np.linalg.inv().

converted linalg.rst to use mat except for schur factorization, expla…
…ined numpy.linalg vs scipi.alg, examplained numpy.ndarray vs numpy.matrix
@cirosantilli

This comment has been minimized.

Contributor

cirosantilli commented Dec 4, 2012

  • converted all examples to use ndarray instead of matrix, except for schur decomposition example
  • added a section explaining numpy.linalg vs scipy.linalg which caused me some confusion when learning
  • added a section exaplaing numpy.matrix vs numpy.ndarray. I know that this is not really strictly scipy related, so I someone thinks this is not the place for that, I understand if you want to remove/shorten that section. However, that point which is quite confusing, and it is the first place where people will look for answers (at least i did =) )
matrix([[17],
[39]])
Dispite its convenience, the use of the ``numpy.matrix`` class is

This comment has been minimized.

@rgommers

rgommers Dec 4, 2012

Member

typo --> Despite

[39]])
Dispite its convenience, the use of the ``numpy.matrix`` class is
discouraged, since it adds nothing to what cannot be accomplished

This comment has been minimized.

@rgommers

rgommers Dec 4, 2012

Member

"to what" --> "that"

spaces separating columns and semicolons separting rows as long as the
matrix is placed in a string passed to :obj:`mat` .
Therefore, unless you don't want to add ``scipy`` as a dependency to
your ``numpy`` program, use ``scipy.linalg`` instead of ``numpy.linalg``

This comment has been minimized.

@rgommers

rgommers Dec 4, 2012

Member

Also note that scipy.linalg is always compiled with BLAS/LAPACK support while for numpy that's optional. So scipy can be much faster, depending on how numpy was installed.

@rgommers

This comment has been minimized.

Member

rgommers commented Dec 4, 2012

Looks good at first sight, I'll have to go through this in more detail later. Adding those two sections is fine with me, they may indeed be helpful for some users.

>>> linalg.norm(A,'fro') #frobenius norm is the default
5.4772255750516612
>>> linalg.norm(A,1) #L1-norm
6

This comment has been minimized.

@fabianp

fabianp Dec 18, 2012

Member

Since A is a matrix this is the operator norm induced by the L1-norm and not the L1-norm. In this case this is the maximum of the sums over columns.

I would replace L1-norm --> max column sum in the comment

>>> linalg.norm(A,-1)
4
>>> linalg.norm(A,inf)
7

This comment has been minimized.

@fabianp

fabianp Dec 18, 2012

Member

a comment # max row sum would be nice.

[[ 1. 3. 2.]
[ 1. 2. 3.]]
>>> Sig = np.array(linalg.diagsvd(s,M,N))
>>> U, Vh = np.array(U), np.array(Vh)

This comment has been minimized.

@fabianp

fabianp Dec 18, 2012

Member

all those np.array are redudant: Sig, U, Vh are already numpy arrays

@fabianp

This comment has been minimized.

Member

fabianp commented Dec 18, 2012

thanks for the contribution @cirosantilli , made a couple of minor comments but looks good otherwise.

@fabianp

This comment has been minimized.

Member

fabianp commented Dec 18, 2012

Also there are a lot of dot products written as A.dot(B). There's nothing wrong with this but what is commonly used in the numpy documentation is np.dot(A, B) instead of A.dot(B). @rgommers : please comment me if I'm wrong about this.

@rgommers

This comment has been minimized.

Member

rgommers commented Dec 18, 2012

That's because dot as a method is a recent addition to numpy. In principle I don't see a problem with using the method instead of the function.

@fabianp

This comment has been minimized.

Member

fabianp commented Dec 21, 2012

OK then I take back my last remark

@rgommers

This comment has been minimized.

Member

rgommers commented Feb 6, 2013

Hi @cirosantilli, would you have time to address the remaining few comments in the next couple of days? Then I'll merge this before creating the next release branch.

@cirosantilli

This comment has been minimized.

Contributor

cirosantilli commented Feb 10, 2013

please tell me if I forgot anything and I'll try to fix it.

@rgommers

This comment has been minimized.

Member

rgommers commented Feb 10, 2013

Looks good to me now, thanks. Merging.

rgommers added a commit that referenced this pull request Feb 10, 2013

Merge pull request #365 from cirosantilli/master
DOC: added norm examples to linalg.rst docs

@rgommers rgommers merged commit 043c5ac into scipy:master Feb 10, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment