Hello.
It is "interesting" to see issue on such old codes...
What you propose looks sensible; the "linear" package has been known
for a long time to need an extensive redesign.
But I'm wondering about unforeseen side-effects of changing some of
those decisions without any possible feedback from those who took
them.
It might be prudent to implement the new approach in a separate
package...
On Mon, 31 Oct 2016 02:16:15 -0700 (PDT), wilbur wrote:
OK, I built 'LUDecomposition' back to match the original Jama version
and
fixed a few things on the way. Performance is consistently much
better and I
did not notice any differences in numerical accuracy. While this
appears to
be the same algorithm, I have not figured out why (though I can see
WHERE)
the current CM implementation wastes so much time.
Working myself through the code I ran into two other issues that
should be
fixed but will change the behavior of this class:
1) The decomposition algorithm works fine even if the input matrix A
is
singular. However, the current implementation exits the constructor
as soon
as singularity is detected and the methods getL(), getU(), and getP()
return
null. I propose to let the decomposition complete even for singular
matrices
and always return a valid decomposition. As usual, the returned
'DecompositionSolver' will raise an exception when solve() is invoked
on a
singular matrix.
2) In the current implementation, the methods getL(), getU(), and
getP()
return references to the SAME (cached) matrix objects in successive
calls.
This appears to be unjustified and bad programming practice, since
RealMatrix objects are mutable. For some reason, though, this is
considered
an important feature, since it is explicitly tested for in the JUnit
test. I
propose to cache the corresponding primitive arrays and return a new
matrix
instance in every call (modified the JUnit test).
Also, I see no reason to make the inner class 'Solver' *static*,
since such
an object cannot exist without the associated LUDecomposition itself.
I this
changed this to a non-static inner class. (Same strange pattern is
used in
QRDecomposition and probably elsewhere.)
My proposed re-implementation can be found here
<https://github.com/WilhelmBurger/commons-math/blob/master/src/main/java/org/apache/commons/math4/linear/LUDecomposition.java>
and the associated JUnit tests here
<https://github.com/WilhelmBurger/commons-math/blob/master/src/test/java/org/apache/commons/math4/linear/LUDecompositionTest.java>
.
TODO: The class 'FieldLUDecomposition' does the same but on a
different
matrix type -- needs to be reworked as well (possibly avoiding code
duplication).
Anyone please let me know in case of any objections or possible
enhancements.
Regards,
Wilhelm
___
PS: Being new to this list I am not sure how to properly create
issues,
since they are not open on GitHub.
The bug-tracking system is here:
https://issues.apache.org/jira/browse/MATH/
Please report your findings there and attach all relevant patches.
Hopefully, someone can review them.
Regards,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
