Commons Math
  1. Commons Math
  2. MATH-1044

Clarify and fix javadoc of DecompositionSolver.getInverse() and implementations

    Details

      Description

      As suggested by Phil on the mailing list, I'm providing a patch to the javadoc of DecompositionSolver.getInverse() to clarify what it does. It also correctly moves the "@throws SingularMatrixException" to implementations that do throw this exception; not all do.

      Tests already cover most of the behavior asserted by the documentation. I added one to show that the SVD does not reject singular matrices. There should be a test for EigenDecomposition here too, but in the course of adding it I found another apparent small bug in its behavior with singular matrices. So I will provide that separately (it actually involves no doc changes, and docs are the topic here.)

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        109d 1h 15m 1 Thomas Neidhart 08/Feb/14 14:16
        Resolved Resolved Closed Closed
        100d 57m 1 Luc Maisonobe 19/May/14 16:13
        Luc Maisonobe made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        Luc Maisonobe added a comment -

        Closing all resolved issue now available in released 3.3 version.

        Show
        Luc Maisonobe added a comment - Closing all resolved issue now available in released 3.3 version.
        Thomas Neidhart made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Thomas Neidhart added a comment -

        Applied the patch in r1566017 with a few minor modifications.

        I decided to still advertise the SingularMatrixException in the interface as I believe it is the right place to do so as the interface should be the common denominator for all possible implementations. Some implementations will not throw this exception in case of a singular matrix, and it is described like that.

        Thanks for the patch!

        Show
        Thomas Neidhart added a comment - Applied the patch in r1566017 with a few minor modifications. I decided to still advertise the SingularMatrixException in the interface as I believe it is the right place to do so as the interface should be the common denominator for all possible implementations. Some implementations will not throw this exception in case of a singular matrix, and it is described like that. Thanks for the patch!
        Thomas Neidhart made changes -
        Fix Version/s 3.3 [ 12324600 ]
        Hide
        Thomas Neidhart added a comment -

        In general it good, but I would still advertise the SingularMatrixException in the DecompositionSolver interface with a @throws clause.
        For each decomposition, I would specify if it can compute a pseudo-inverse or will throw an exception in case of a singular matrix.

        The reason being that a user might just use an interface and may not know explicitly the used implementation, thus must protect itself against anything that can happen, including the SingularMatrixException.

        btw. I would also add the specific inverse behavior, as mentioned before, to the class javadoc of each decomposition, as this one will be more often read as the one from the embedded DecompositionSolver imho.

        Show
        Thomas Neidhart added a comment - In general it good, but I would still advertise the SingularMatrixException in the DecompositionSolver interface with a @throws clause. For each decomposition, I would specify if it can compute a pseudo-inverse or will throw an exception in case of a singular matrix. The reason being that a user might just use an interface and may not know explicitly the used implementation, thus must protect itself against anything that can happen, including the SingularMatrixException. btw. I would also add the specific inverse behavior, as mentioned before, to the class javadoc of each decomposition, as this one will be more often read as the one from the embedded DecompositionSolver imho.
        Hide
        Phil Steitz added a comment -

        This looks accurate to me. Any objections to committing the patch?

        Show
        Phil Steitz added a comment - This looks accurate to me. Any objections to committing the patch?
        Sean Owen made changes -
        Attachment MATH-1044.patch [ 12609646 ]
        Sean Owen made changes -
        Attachment MATH-1044.patch [ 12609657 ]
        Hide
        Sean Owen added a comment -

        Now with LaTeX

        Show
        Sean Owen added a comment - Now with LaTeX
        Hide
        Gilles added a comment -

        If you know (and like) LaTeX, it is now possible to write formulae with it within the Javadoc (thanks to MathJax).
        Interpretation is activated by having an inline formula enclosed between

        \(

        and

        \)

        and a display formula enclosed between

        \[

        and

        \]

        See for example the doc of class org.apache.commons.math3.fitting.HarmonicCurveFitter.

        Show
        Gilles added a comment - If you know (and like) LaTeX, it is now possible to write formulae with it within the Javadoc (thanks to MathJax). Interpretation is activated by having an inline formula enclosed between \( and \) and a display formula enclosed between \[ and \] See for example the doc of class org.apache.commons.math3.fitting.HarmonicCurveFitter .
        Sean Owen made changes -
        Attachment MATH-1045.patch [ 12609647 ]
        Sean Owen made changes -
        Attachment MATH-1045.patch [ 12609647 ]
        Sean Owen made changes -
        Field Original Value New Value
        Attachment MATH-1044.patch [ 12609646 ]
        Sean Owen created issue -

          People

          • Assignee:
            Unassigned
            Reporter:
            Sean Owen
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development