Re: [MATH] Repurposing a deprecated constructor in EigenDecomposition
On 10/23/13 8:03 PM, Ted Dunning wrote: On Wed, Oct 23, 2013 at 8:33 PM, Sean Owen sro...@gmail.com wrote: it feels a little funny just because then we should have similar logic for other decompositions. I think I remember the LU one stops early, always. The stopping early is definitely an option with QR. With LU, it isn't so clear. Sorry to be a little late to the party on this. We should keep in mind that singularity threshold means different things in the different contexts discussed above (as Ted and others have mentioned). It should be provided to constructors that need it and clearly documented (cf LU). I like adding a boolean to Eigen so the user can decide whether or not to stop. I agree with Thomas that adding a parameter to getSolver makes sense in cases where there is no instance field (or even possible where there is, so the solver can override). What really needs to be improved is the documentation on what exactly threshold means in each case where it is used. So basically, I am saying 0) get rid of constructor argument (i.e. depecate) where it is only used by getSolver (QR), adding it there. Look at all of them to see if it makes sense to provide this parameter. 1) work around the overloading issue in EigenDecomposiiton by adding the boolean flag (which is useful anyway) 2) crisp up doco throughout to make it clear exactly what thresholds mean Phil - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
[MATH] Repurposing a deprecated constructor in EigenDecomposition
In MATH-1045 (https://issues.apache.org/jira/browse/MATH-1045) we have discussed adding a zero threshold tolerance to EigenDecomposition just like QRDecomposition has. This involves adding a new constructor with a new double parameter. Just one problem: there's already such a constructor: /** * Calculates the eigen decomposition of the given real matrix. * * @param matrix Matrix to decompose. * @param splitTolerance Dummy parameter (present for backward * compatibility only). * @throws MathArithmeticException if the decomposition of a general matrix * results in a matrix with zero norm * @throws MaxCountExceededException if the algorithm fails to converge. * @deprecated in 3.1 (to be removed in 4.0) due to unused parameter */ @Deprecated public EigenDecomposition(final RealMatrix matrix, final double splitTolerance) throws MathArithmeticException { this(matrix); } Reusing this constructor would introduce a functionally incompatible change, as the previous meaning of this value was different. What are people's thoughts on proceeding anyway for a minor point release like 3.3? Or, best saved for 4.x? Sean - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [MATH] Repurposing a deprecated constructor in EigenDecomposition
Yeah it's a tricky one. I agree with the functionality, because the QR decomposition can happily operate on a singular matrix. R will be singular if A is, AFAIK. So the singularity threshold business belongs more in the bit that uses R, yes. The same could be said of the SVD. But it provides no singularity threshold parameter. Not suggesting changing that here, let's put it aside. Cholesky doesn't need a singularity threshold, but takes other thresholds in its constructor. That's OK since they are relevant to the decomposition. Same for LU -- it needs a singularity threshold because the LU decomposition can proceed but only to a point if the matrix is singular. (This is at the limit of my understanding.) EigenDecomposition resembles QR in this respect, as far as they are implemented here. This argues for them to treat arguments similarly. I suppose the best thing would be to move the threshold to getSolver() in both cases. This seems a better design, and no more trouble than, dealing with the problem of the old deprecated constructor and putting the arguments in the decomposition constructor in both places. I suppose it's also reasonable to punt and not add this threshold just now. Any more thoughts? On Wed, Oct 23, 2013 at 1:14 PM, Thomas Neidhart thomas.neidh...@gmail.com wrote: Hi Sean, while looking at the QRDecomposition, it is interesting that the threshold value provided to the constructor of QRDecomposition is only used to pass it on the the Solver created by getSolver(). Imho, it would be more logical to chose the singularity threshold when calling getSolver() for the respective decomposition, that way we could also add it to the EigenDecomposition class. Thomas On Wed, Oct 23, 2013 at 1:23 PM, Sean Owen sro...@gmail.com wrote: In MATH-1045 (https://issues.apache.org/jira/browse/MATH-1045) we have discussed adding a zero threshold tolerance to EigenDecomposition just like QRDecomposition has. This involves adding a new constructor with a new double parameter. Just one problem: there's already such a constructor: /** * Calculates the eigen decomposition of the given real matrix. * * @param matrix Matrix to decompose. * @param splitTolerance Dummy parameter (present for backward * compatibility only). * @throws MathArithmeticException if the decomposition of a general matrix * results in a matrix with zero norm * @throws MaxCountExceededException if the algorithm fails to converge. * @deprecated in 3.1 (to be removed in 4.0) due to unused parameter */ @Deprecated public EigenDecomposition(final RealMatrix matrix, final double splitTolerance) throws MathArithmeticException { this(matrix); } Reusing this constructor would introduce a functionally incompatible change, as the previous meaning of this value was different. What are people's thoughts on proceeding anyway for a minor point release like 3.3? Or, best saved for 4.x? Sean - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [MATH] Repurposing a deprecated constructor in EigenDecomposition
On Wed, Oct 23, 2013 at 3:14 PM, Sean Owen sro...@gmail.com wrote: EigenDecomposition resembles QR in this respect, as far as they are implemented here. This argues for them to treat arguments similarly. Actually not. It is quite reasonable for the EigenDecomposition to stop when singularity is reached. This affects the shape of the eigenvector matrix. Perhaps add a new constructor with a double tolerance and a boolean that says to stop early. QR is subject to the same logic since partial QR is often more useful than full QR with singular R. This is the same logic as with Cholesky since QR and Cholesky are two sides of the same coin in many respects.
Re: [MATH] Repurposing a deprecated constructor in EigenDecomposition
Ah I see so you can just stop computing the eigenvectors when the eigenvalues are too small. That does seem more efficient and then the zero values are left as literally 0. I think I should back up and implement the patch that way. The original question stands then -- what to do about the singularity threshold parameter given the existence of a constructor with the signature already? because that is where it belongs. Adding an extra boolean flag dodges that question; it feels a little funny just because then we should have similar logic for other decompositions. I think I remember the LU one stops early, always. On Wed, Oct 23, 2013 at 4:50 PM, Ted Dunning ted.dunn...@gmail.com wrote: On Wed, Oct 23, 2013 at 3:14 PM, Sean Owen sro...@gmail.com wrote: EigenDecomposition resembles QR in this respect, as far as they are implemented here. This argues for them to treat arguments similarly. Actually not. It is quite reasonable for the EigenDecomposition to stop when singularity is reached. This affects the shape of the eigenvector matrix. Perhaps add a new constructor with a double tolerance and a boolean that says to stop early. QR is subject to the same logic since partial QR is often more useful than full QR with singular R. This is the same logic as with Cholesky since QR and Cholesky are two sides of the same coin in many respects. - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [MATH] Repurposing a deprecated constructor in EigenDecomposition
On Wed, Oct 23, 2013 at 8:33 PM, Sean Owen sro...@gmail.com wrote: it feels a little funny just because then we should have similar logic for other decompositions. I think I remember the LU one stops early, always. The stopping early is definitely an option with QR. With LU, it isn't so clear.