Re: [Numpy-discussion] Dealing with the mode argument in qr.

2013-02-06 Thread Benjamin Root
On Tue, Feb 5, 2013 at 4:23 PM, Charles R Harris
charlesr.har...@gmail.comwrote:

 Hi All,

 This post is to bring the discussion of PR 
 #2965https://github.com/numpy/numpy/pull/2965to the attention of the list. 
 There are at least three issues in play here.

 1) The PR adds modes 'big' and 'thin' to the current modes 'full', 'r',
 'economic' for qr factorization. The problem is that the current 'full' is
 actually 'thin' and 'big' should be 'full'. The solution here was to raise
 a FutureWarning on use of 'full', alias it to 'thin' for the time being,
 and at some distant time change 'full' to alias 'big'.

 2) The 'economic' mode serves little purpose. I propose to deprecate it
 and add a 'qrf' mode instead, corresponding to scipy's 'raw' mode. We can't
 use 'raw' itself as traditionally the mode may be specified using the first
 letter only and that leads to a conflict with 'r'.

 3) As suggested in 2, the use of single letter abbreviations can constrain
 the options in choosing mode names and they are not as informative as the
 full name. A possibility here is to deprecate the use of the abbreviations
 in favor of the full names.

 A longer term problem is the divergence between the numpy and scipy
 versions of qr. The divergence is enough that I don't see any easy way to
 come to a common interface, but that is something that would be desirable
 if possible.

 Thoughts?

 Chuck


I would definitely be in favor of deprecating abbreviations.

And while we are on the topic of mode names,
scipy.ndimage.filters.percentile_filter() has modes of 'mirror' and
'reflect', and I don't see any documentation stating if they are the same,
or what are different about them.  I just came across this yesterday.

Cheers!
Ben Root
___
NumPy-Discussion mailing list
NumPy-Discussion@scipy.org
http://mail.scipy.org/mailman/listinfo/numpy-discussion


Re: [Numpy-discussion] Dealing with the mode argument in qr.

2013-02-06 Thread Ralf Gommers
On Wed, Feb 6, 2013 at 3:44 PM, Benjamin Root ben.r...@ou.edu wrote:



 On Tue, Feb 5, 2013 at 4:23 PM, Charles R Harris 
 charlesr.har...@gmail.com wrote:

 Hi All,

 This post is to bring the discussion of PR 
 #2965https://github.com/numpy/numpy/pull/2965to the attention of the list. 
 There are at least three issues in play here.

 1) The PR adds modes 'big' and 'thin' to the current modes 'full', 'r',
 'economic' for qr factorization. The problem is that the current 'full' is
 actually 'thin' and 'big' should be 'full'. The solution here was to raise
 a FutureWarning on use of 'full', alias it to 'thin' for the time being,
 and at some distant time change 'full' to alias 'big'.


This is asking for problems, to gain some naming consistency. I can't tell
how confusing 'full' is now, but deprecating and removing would be better
than changing what it returns.



 2) The 'economic' mode serves little purpose. I propose to deprecate it
 and add a 'qrf' mode instead, corresponding to scipy's 'raw' mode. We can't
 use 'raw' itself as traditionally the mode may be specified using the first
 letter only and that leads to a conflict with 'r'.


That's not a very good reason to not use raw, since raw is a new option
and you therefore don't have to apply the rule that you can give only the
first letter to it.



 3) As suggested in 2, the use of single letter abbreviations can
 constrain the options in choosing mode names and they are not as
 informative as the full name. A possibility here is to deprecate the use of
 the abbreviations in favor of the full names.


I'm not feeling very strongly about this, but we have to be careful about
deprecations. Possible future naming constraints on new modes is not a good
reason to deprecate. This one-letter option isn't even mentioned in the
docs it looks like. So why not leave that as is and ensure it keeps working
(add a unit test if needed)?



 A longer term problem is the divergence between the numpy and scipy
 versions of qr. The divergence is enough that I don't see any easy way to
 come to a common interface, but that is something that would be desirable
 if possible.


This would be a problem imho. But I don't see why you can't add raw to
numpy's qr. And if you add big and thin in numpy, you can add those
modes in scipy too.

Ralf



 Thoughts?

 Chuck


 I would definitely be in favor of deprecating abbreviations.

 And while we are on the topic of mode names,
 scipy.ndimage.filters.percentile_filter() has modes of 'mirror' and
 'reflect', and I don't see any documentation stating if they are the same,
 or what are different about them.  I just came across this yesterday.

 Cheers!
 Ben Root


 ___
 NumPy-Discussion mailing list
 NumPy-Discussion@scipy.org
 http://mail.scipy.org/mailman/listinfo/numpy-discussion


___
NumPy-Discussion mailing list
NumPy-Discussion@scipy.org
http://mail.scipy.org/mailman/listinfo/numpy-discussion


Re: [Numpy-discussion] Dealing with the mode argument in qr.

2013-02-06 Thread Charles R Harris
On Wed, Feb 6, 2013 at 12:17 PM, Ralf Gommers ralf.gomm...@gmail.comwrote:




 On Wed, Feb 6, 2013 at 3:44 PM, Benjamin Root ben.r...@ou.edu wrote:



 On Tue, Feb 5, 2013 at 4:23 PM, Charles R Harris 
 charlesr.har...@gmail.com wrote:

 Hi All,

 This post is to bring the discussion of PR 
 #2965https://github.com/numpy/numpy/pull/2965to the attention of the 
 list. There are at least three issues in play here.

 1) The PR adds modes 'big' and 'thin' to the current modes 'full', 'r',
 'economic' for qr factorization. The problem is that the current 'full' is
 actually 'thin' and 'big' should be 'full'. The solution here was to raise
 a FutureWarning on use of 'full', alias it to 'thin' for the time being,
 and at some distant time change 'full' to alias 'big'.


 This is asking for problems, to gain some naming consistency. I can't tell
 how confusing 'full' is now, but deprecating and removing would be better
 than changing what it returns.


That's what the current state of the PR is, both 'full' and 'economic' are
deprecated.





 2) The 'economic' mode serves little purpose. I propose to deprecate it
 and add a 'qrf' mode instead, corresponding to scipy's 'raw' mode. We can't
 use 'raw' itself as traditionally the mode may be specified using the first
 letter only and that leads to a conflict with 'r'.


 That's not a very good reason to not use raw, since raw is a new
 option and you therefore don't have to apply the rule that you can give
 only the first letter to it.


Also the current state.





 3) As suggested in 2, the use of single letter abbreviations can
 constrain the options in choosing mode names and they are not as
 informative as the full name. A possibility here is to deprecate the use of
 the abbreviations in favor of the full names.


 I'm not feeling very strongly about this, but we have to be careful about
 deprecations. Possible future naming constraints on new modes is not a good
 reason to deprecate. This one-letter option isn't even mentioned in the
 docs it looks like. So why not leave that as is and ensure it keeps working
 (add a unit test if needed)?


Currently qr requires full names for the new modes but not for the
deprecated 'full' and 'economic'. That can be changed if we use 'thin'
instead of 'reduced'.




 A longer term problem is the divergence between the numpy and scipy
 versions of qr. The divergence is enough that I don't see any easy way to
 come to a common interface, but that is something that would be desirable
 if possible.


 This would be a problem imho. But I don't see why you can't add raw to
 numpy's qr. And if you add big and thin in numpy, you can add those
 modes in scipy too.


Currently I've used bfroehle's suggestions, although I'm tempted by 'thin'
instead of 'reduced'

snip

Chuck
___
NumPy-Discussion mailing list
NumPy-Discussion@scipy.org
http://mail.scipy.org/mailman/listinfo/numpy-discussion


Re: [Numpy-discussion] Dealing with the mode argument in qr.

2013-02-06 Thread Bradley M. Froehle
 This would be a problem imho. But I don't see why you can't add raw to
 numpy's qr. And if you add big and thin in numpy, you can add those
 modes in scipy too.


 Currently I've used bfroehle's suggestions, although I'm tempted by 'thin'
 instead of 'reduced'


Thin sounds fine to me.  Either way I think we need to clean up the
docstring to make the different calling styles more obvious.  Perhaps we
can just add a quick list of variants:
q, r = qr(a) # q is m-by-k, r is k-by-n
q, r = qr(a, 'thin')  # same as qr(a)
q, r = qr(a, 'complete') # q is m-by-n, r is n-by-n
   r = qr(a, 'r') # r is 
a2  = qr(a, 'economic') # r is contained in the upper triangular part of a2
a2, tau = qr(a, 'raw') # ...

Regards,
Brad
___
NumPy-Discussion mailing list
NumPy-Discussion@scipy.org
http://mail.scipy.org/mailman/listinfo/numpy-discussion


Re: [Numpy-discussion] Dealing with the mode argument in qr.

2013-02-06 Thread Charles R Harris
On Wed, Feb 6, 2013 at 1:38 PM, Bradley M. Froehle
brad.froe...@gmail.comwrote:


  This would be a problem imho. But I don't see why you can't add raw to
 numpy's qr. And if you add big and thin in numpy, you can add those
 modes in scipy too.


 Currently I've used bfroehle's suggestions, although I'm tempted by
 'thin' instead of 'reduced'


 Thin sounds fine to me.  Either way I think we need to clean up the
 docstring to make the different calling styles more obvious.  Perhaps we
 can just add a quick list of variants:
 q, r = qr(a) # q is m-by-k, r is k-by-n
 q, r = qr(a, 'thin')  # same as qr(a)
 q, r = qr(a, 'complete') # q is m-by-n, r is n-by-n
r = qr(a, 'r') # r is 
 a2  = qr(a, 'economic') # r is contained in the upper triangular part of a2
 a2, tau = qr(a, 'raw') # ...


There is a list similar to that already there. Take a look and comment.

Chuck



___
NumPy-Discussion mailing list
NumPy-Discussion@scipy.org
http://mail.scipy.org/mailman/listinfo/numpy-discussion


Re: [Numpy-discussion] Dealing with the mode argument in qr.

2013-02-05 Thread Charles R Harris
On Tue, Feb 5, 2013 at 2:23 PM, Charles R Harris
charlesr.har...@gmail.comwrote:

 Hi All,

 This post is to bring the discussion of PR 
 #2965https://github.com/numpy/numpy/pull/2965to the attention of the list. 
 There are at least three issues in play here.

 1) The PR adds modes 'big' and 'thin' to the current modes 'full', 'r',
 'economic' for qr factorization. The problem is that the current 'full' is
 actually 'thin' and 'big' should be 'full'. The solution here was to raise
 a FutureWarning on use of 'full', alias it to 'thin' for the time being,
 and at some distant time change 'full' to alias 'big'.

 2) The 'economic' mode serves little purpose. I propose to deprecate it
 and add a 'qrf' mode instead, corresponding to scipy's 'raw' mode. We can't
 use 'raw' itself as traditionally the mode may be specified using the first
 letter only and that leads to a conflict with 'r'.

 3) As suggested in 2, the use of single letter abbreviations can constrain
 the options in choosing mode names and they are not as informative as the
 full name. A possibility here is to deprecate the use of the abbreviations
 in favor of the full names.

 A longer term problem is the divergence between the numpy and scipy
 versions of qr. The divergence is enough that I don't see any easy way to
 come to a common interface, but that is something that would be desirable
 if possible.

 Thoughts?


bfroehle has suggested the names

1. complete: Q is a M-by-M matrix, i.e. a complete orthogonal basis.
2. reduced: Q is a M-by-K matrix.
3. r: Only return R
4. raw: Return Householder reflectors Q and TAU

Chuck
___
NumPy-Discussion mailing list
NumPy-Discussion@scipy.org
http://mail.scipy.org/mailman/listinfo/numpy-discussion