You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Gilles (JIRA)" <ji...@apache.org> on 2011/02/21 23:52:39 UTC

[jira] Created: (MATH-521) Changes in "HarmonicCoefficientsGuesser"

Changes in "HarmonicCoefficientsGuesser"
----------------------------------------

                 Key: MATH-521
                 URL: https://issues.apache.org/jira/browse/MATH-521
             Project: Commons Math
          Issue Type: Improvement
            Reporter: Gilles
            Assignee: Gilles
            Priority: Minor
             Fix For: 3.0


(1) The "guess" method throws "OptimizationException" when the algorithm fails to determine valid values for amplitude and angular frequency.
There are no test showing how such a situation can occur.
Moreover, since this procedure is used to provide an initial guess to an optimizer, it is better to pick any values for those parameters (e.g. zero) and let the optimizer proceed from that initial point.

(2) The class javadoc seems very thorough in explaining the algorithm, but is quite unreadable in the source code, making it fairly useless for checking how the code complies with the comments. I think that this explanation should go in the user guide (and leave a mostly "plain text" outline of the algorithm, referring to the guide for details). [Does the format of the user guide allow such tricky (ASCII "art") constructs?]


-- 
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Commented: (MATH-521) Changes in "HarmonicCoefficientsGuesser"

Posted by "Gilles (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MATH-521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12997775#comment-12997775 ] 

Gilles commented on MATH-521:
-----------------------------

{quote}
The javadoc is for sure not perfect but is readable [...]
{quote}

There should be a balance between "nice" graphics in a browser and readability _within_ the source code.
In this particular case, it is more important that a developer can read the algorithm description _in the source file_ because most users that will read the API doc of that class rendered in a browser, will

# not call it directly, and
# be interested in how to use it, not how it works.


> Changes in "HarmonicCoefficientsGuesser"
> ----------------------------------------
>
>                 Key: MATH-521
>                 URL: https://issues.apache.org/jira/browse/MATH-521
>             Project: Commons Math
>          Issue Type: Improvement
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Minor
>              Labels: api-change, documentation
>             Fix For: 3.0
>
>
> (1) The "guess" method throws "OptimizationException" when the algorithm fails to determine valid values for amplitude and angular frequency.
> There are no test showing how such a situation can occur.
> Moreover, since this procedure is used to provide an initial guess to an optimizer, it is better to pick any values for those parameters (e.g. zero) and let the optimizer proceed from that initial point.
> (2) The class javadoc seems very thorough in explaining the algorithm, but is quite unreadable in the source code, making it fairly useless for checking how the code complies with the comments. I think that this explanation should go in the user guide (and leave a mostly "plain text" outline of the algorithm, referring to the guide for details). [Does the format of the user guide allow such tricky (ASCII "art") constructs?]

-- 
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Commented: (MATH-521) Changes in "HarmonicCoefficientsGuesser"

Posted by "Luc Maisonobe (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MATH-521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12997696#comment-12997696 ] 

Luc Maisonobe commented on MATH-521:
------------------------------------

Do you want to remove the guess method or to have default values just when the guess fails ? We should add tests for such failures. I think one way to make it fail would be for example to supply a constant sample with 0 values only for y, or a set of points with only one value for x and random values for y. 

I think for the frequency part, 0 is probably not a good choice and would lead to numerical problems. Perhaps a value like 2PI/l where l is the range in X would be better, it would start considering the sample covers exactly one period.

The javadoc is for sure not perfect but is readable in a browser, which is the main intend. The output is here: [http://commons.apache.org/math/api-2.1/org/apache/commons/math/optimization/fitting/HarmonicCoefficientsGuesser.html].

Putting this in the user guide would be a nice addition.

> Changes in "HarmonicCoefficientsGuesser"
> ----------------------------------------
>
>                 Key: MATH-521
>                 URL: https://issues.apache.org/jira/browse/MATH-521
>             Project: Commons Math
>          Issue Type: Improvement
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Minor
>              Labels: api-change, documentation
>             Fix For: 3.0
>
>
> (1) The "guess" method throws "OptimizationException" when the algorithm fails to determine valid values for amplitude and angular frequency.
> There are no test showing how such a situation can occur.
> Moreover, since this procedure is used to provide an initial guess to an optimizer, it is better to pick any values for those parameters (e.g. zero) and let the optimizer proceed from that initial point.
> (2) The class javadoc seems very thorough in explaining the algorithm, but is quite unreadable in the source code, making it fairly useless for checking how the code complies with the comments. I think that this explanation should go in the user guide (and leave a mostly "plain text" outline of the algorithm, referring to the guide for details). [Does the format of the user guide allow such tricky (ASCII "art") constructs?]

-- 
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Commented: (MATH-521) Changes in "HarmonicCoefficientsGuesser"

Posted by "Gilles (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MATH-521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12998423#comment-12998423 ] 

Gilles commented on MATH-521:
-----------------------------

I've added 2 test cases in revision 1073796.

One covers the "guesser" code when the original algorithm fails. However it is not a very robust test (nothing is actually tested).

The second is commented out (because it doesn't work). Maybe the initial algorithm should check earlier that something is wrong with the input data (a single "x" value with different "y").


> Changes in "HarmonicCoefficientsGuesser"
> ----------------------------------------
>
>                 Key: MATH-521
>                 URL: https://issues.apache.org/jira/browse/MATH-521
>             Project: Commons Math
>          Issue Type: Improvement
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Minor
>              Labels: api-change, documentation
>             Fix For: 3.0
>
>         Attachments: example_LaTeXLet.tar.gz
>
>
> (1) The "guess" method throws "OptimizationException" when the algorithm fails to determine valid values for amplitude and angular frequency.
> There are no test showing how such a situation can occur.
> Moreover, since this procedure is used to provide an initial guess to an optimizer, it is better to pick any values for those parameters (e.g. zero) and let the optimizer proceed from that initial point.
> (2) The class javadoc seems very thorough in explaining the algorithm, but is quite unreadable in the source code, making it fairly useless for checking how the code complies with the comments. I think that this explanation should go in the user guide (and leave a mostly "plain text" outline of the algorithm, referring to the guide for details). [Does the format of the user guide allow such tricky (ASCII "art") constructs?]

-- 
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Commented: (MATH-521) Changes in "HarmonicCoefficientsGuesser"

Posted by "Luc Maisonobe (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MATH-521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12997792#comment-12997792 ] 

Luc Maisonobe commented on MATH-521:
------------------------------------

For sure this is much nicer than my ASCII art, both in source and in browser.
However, a GPL V3 tool, with a requirement that a complete LaTeX installation and dvipng are installed for the build phase is far too much.

> Changes in "HarmonicCoefficientsGuesser"
> ----------------------------------------
>
>                 Key: MATH-521
>                 URL: https://issues.apache.org/jira/browse/MATH-521
>             Project: Commons Math
>          Issue Type: Improvement
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Minor
>              Labels: api-change, documentation
>             Fix For: 3.0
>
>         Attachments: example_LaTeXLet.tar.gz
>
>
> (1) The "guess" method throws "OptimizationException" when the algorithm fails to determine valid values for amplitude and angular frequency.
> There are no test showing how such a situation can occur.
> Moreover, since this procedure is used to provide an initial guess to an optimizer, it is better to pick any values for those parameters (e.g. zero) and let the optimizer proceed from that initial point.
> (2) The class javadoc seems very thorough in explaining the algorithm, but is quite unreadable in the source code, making it fairly useless for checking how the code complies with the comments. I think that this explanation should go in the user guide (and leave a mostly "plain text" outline of the algorithm, referring to the guide for details). [Does the format of the user guide allow such tricky (ASCII "art") constructs?]

-- 
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Updated: (MATH-521) Changes in "HarmonicCoefficientsGuesser"

Posted by "Gilles (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MATH-521?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Gilles updated MATH-521:
------------------------

    Attachment: example_LaTeXLet.tar.gz

This is an example IMO clearly demonstrating that a LaTeX equivalent of those ASCII structures would be

# incomparably easier to decypher in the source file, and
# be even more pleasant to read in the browser as a graphics file rendered by LaTeX.

I've just discovered "LaTeXLet" a tool that does what I had talked about on the ML a few weeks ago: A custom taglet that allows LaTeX in Javadoc and renders it as PNG.

Please have a look at the generated HTML doc (in the attachement) when run on the input file "DocSample.java" (also included).


> Changes in "HarmonicCoefficientsGuesser"
> ----------------------------------------
>
>                 Key: MATH-521
>                 URL: https://issues.apache.org/jira/browse/MATH-521
>             Project: Commons Math
>          Issue Type: Improvement
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Minor
>              Labels: api-change, documentation
>             Fix For: 3.0
>
>         Attachments: example_LaTeXLet.tar.gz
>
>
> (1) The "guess" method throws "OptimizationException" when the algorithm fails to determine valid values for amplitude and angular frequency.
> There are no test showing how such a situation can occur.
> Moreover, since this procedure is used to provide an initial guess to an optimizer, it is better to pick any values for those parameters (e.g. zero) and let the optimizer proceed from that initial point.
> (2) The class javadoc seems very thorough in explaining the algorithm, but is quite unreadable in the source code, making it fairly useless for checking how the code complies with the comments. I think that this explanation should go in the user guide (and leave a mostly "plain text" outline of the algorithm, referring to the guide for details). [Does the format of the user guide allow such tricky (ASCII "art") constructs?]

-- 
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Commented: (MATH-521) Changes in "HarmonicCoefficientsGuesser"

Posted by "Gilles (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MATH-521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12997815#comment-12997815 ] 

Gilles commented on MATH-521:
-----------------------------

Why the remark about GPL? This is not a code contribution.

Obviously, to render a LaTeX source, you need a LaTeX installation!

I hope that you don't consider this requirement as a "dependency": The CM code will not need "latex" to run...
Neither are "latex" and "dvipng" needed to read the Javadoc; they are needed only on the machine that builds the documentation. It's not even a mandatory requirement: "javadoc" can generate the doc without the taglet: It will display warnings and just not render the "unknown" tags.

This tool is about as easy as can be; hence this option is quite worth investigating further, especially since CM also aims at high-quality documentation. In the scientific domain, this goes together with LaTeX formatting.

We can have a documentation-writing rule stating that the possibility of LaTeX formatting should not be abused: For example, that the standard tags ("@param", "@return", "@throws", etc.) should not use it, but that it can be used to avoid the kind of unreadable documentation we have in this class.


> Changes in "HarmonicCoefficientsGuesser"
> ----------------------------------------
>
>                 Key: MATH-521
>                 URL: https://issues.apache.org/jira/browse/MATH-521
>             Project: Commons Math
>          Issue Type: Improvement
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Minor
>              Labels: api-change, documentation
>             Fix For: 3.0
>
>         Attachments: example_LaTeXLet.tar.gz
>
>
> (1) The "guess" method throws "OptimizationException" when the algorithm fails to determine valid values for amplitude and angular frequency.
> There are no test showing how such a situation can occur.
> Moreover, since this procedure is used to provide an initial guess to an optimizer, it is better to pick any values for those parameters (e.g. zero) and let the optimizer proceed from that initial point.
> (2) The class javadoc seems very thorough in explaining the algorithm, but is quite unreadable in the source code, making it fairly useless for checking how the code complies with the comments. I think that this explanation should go in the user guide (and leave a mostly "plain text" outline of the algorithm, referring to the guide for details). [Does the format of the user guide allow such tricky (ASCII "art") constructs?]

-- 
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Resolved] (MATH-521) Changes in "HarmonicCoefficientsGuesser"

Posted by "Gilles (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/MATH-521?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Gilles resolved MATH-521.
-------------------------

    Resolution: Fixed

> Changes in "HarmonicCoefficientsGuesser"
> ----------------------------------------
>
>                 Key: MATH-521
>                 URL: https://issues.apache.org/jira/browse/MATH-521
>             Project: Commons Math
>          Issue Type: Improvement
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Minor
>              Labels: api-change, documentation
>             Fix For: 3.0
>
>         Attachments: example_LaTeXLet.tar.gz
>
>
> (1) The "guess" method throws "OptimizationException" when the algorithm fails to determine valid values for amplitude and angular frequency.
> There are no test showing how such a situation can occur.
> Moreover, since this procedure is used to provide an initial guess to an optimizer, it is better to pick any values for those parameters (e.g. zero) and let the optimizer proceed from that initial point.
> (2) The class javadoc seems very thorough in explaining the algorithm, but is quite unreadable in the source code, making it fairly useless for checking how the code complies with the comments. I think that this explanation should go in the user guide (and leave a mostly "plain text" outline of the algorithm, referring to the guide for details). [Does the format of the user guide allow such tricky (ASCII "art") constructs?]

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] Commented: (MATH-521) Changes in "HarmonicCoefficientsGuesser"

Posted by "Gilles (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/MATH-521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12997773#comment-12997773 ] 

Gilles commented on MATH-521:
-----------------------------

When the guess algorithm fails, it should nevertheless provide default initial values (instead of bailing out).
I'll try to set up a unit test along the lines you indicated.


> Changes in "HarmonicCoefficientsGuesser"
> ----------------------------------------
>
>                 Key: MATH-521
>                 URL: https://issues.apache.org/jira/browse/MATH-521
>             Project: Commons Math
>          Issue Type: Improvement
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Minor
>              Labels: api-change, documentation
>             Fix For: 3.0
>
>
> (1) The "guess" method throws "OptimizationException" when the algorithm fails to determine valid values for amplitude and angular frequency.
> There are no test showing how such a situation can occur.
> Moreover, since this procedure is used to provide an initial guess to an optimizer, it is better to pick any values for those parameters (e.g. zero) and let the optimizer proceed from that initial point.
> (2) The class javadoc seems very thorough in explaining the algorithm, but is quite unreadable in the source code, making it fairly useless for checking how the code complies with the comments. I think that this explanation should go in the user guide (and leave a mostly "plain text" outline of the algorithm, referring to the guide for details). [Does the format of the user guide allow such tricky (ASCII "art") constructs?]

-- 
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira