[jira] [Commented] (MATH-677) About package transform
[
https://issues.apache.org/jira/browse/MATH-677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13197122#comment-13197122
]
Luc Maisonobe commented on MATH-677:
These changes sounds good to me.
About package transform
-
Key: MATH-677
URL: https://issues.apache.org/jira/browse/MATH-677
Project: Commons Math
Issue Type: Improvement
Reporter: Gilles
Assignee: Sébastien Brisard
Priority: Minor
Labels: api-change
Fix For: 3.0
Classes in package o.a.c.m.transform might require some changes in order to
conform to goals set for the next major release.
Some observations:
# Exceptions {color:red}done (see below){color}
## Should remove use of deprecated MathRuntimeException
## Should throw more specific Math...Exception instances instead of
standard IAE
# Interface RealTransformer (and implementations) contain non-conformant
method names (e.g. inversetransform instead of inverseTransform).
{color:red}Fixed in {{r1208293}}.{color}
# FastFourierTransformer:
## Methods mdfft and verifyDataSet take an argument of type Object (to
allow an argument with an unspecified number of dimensions)
## The RootsOfUnity helper class could be moved to the complex package.
{color:red}Done in {{r1237544}}.{color} New class needs a little bit of
cleaning up (as well as unit tests), see below.
## For clarity, multidimensional transform should be moved to a class of its
own (and I also wonder whether the MultiDimensionalComplexMatrix name is
not misleading)
# FastFourierTransformer, FastSineTranformer and FastCosineTranformer
define public methods tranform2 and inversetransform2 but they are not
part of an interface. {color:red}As of {{r1213157}}, these methods have been
removed, and replaced by factory methods {{create()}} and {{createUnitary()}}
(FFT) or {{createOrthogonal()}} (FCT, FST).{color}
# Code uses variables that start with an uppercase. {color:red}Fixed,
together with various formatting issues.{color}
# FastHadamardTransformer contains illegible developer documentation (see
Javadoc for protected method fht). {color:red}Tried to improve things in
{{r1208986}}, but things are still a bit obscure. Besides, the link provided
is broken. Will look into that.{color}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-677) About package transform
[
https://issues.apache.org/jira/browse/MATH-677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13197497#comment-13197497
]
Sébastien Brisard commented on MATH-677:
In {{r1238898}}
* renamed {{computeOmega(int)}} to {{computeRoots(int)}}
* renamed {{getOmegaReal(int)}} to {{getReal(int)}}
* renamed {{getOmegaImaginary(int)}} to {{getImaginary(int)}}
* added {{int getNumberOfRoots()}}
* added unit tests.
About package transform
-
Key: MATH-677
URL: https://issues.apache.org/jira/browse/MATH-677
Project: Commons Math
Issue Type: Improvement
Reporter: Gilles
Assignee: Sébastien Brisard
Priority: Minor
Labels: api-change
Fix For: 3.0
Classes in package o.a.c.m.transform might require some changes in order to
conform to goals set for the next major release.
Some observations:
# Exceptions {color:red}done (see below){color}
## Should remove use of deprecated MathRuntimeException
## Should throw more specific Math...Exception instances instead of
standard IAE
# Interface RealTransformer (and implementations) contain non-conformant
method names (e.g. inversetransform instead of inverseTransform).
{color:red}Fixed in {{r1208293}}.{color}
# FastFourierTransformer:
## Methods mdfft and verifyDataSet take an argument of type Object (to
allow an argument with an unspecified number of dimensions)
## The RootsOfUnity helper class could be moved to the complex package.
{color:red}Done in {{r1237544}}.{color} New class needs a little bit of
cleaning up (as well as unit tests), see below.
## For clarity, multidimensional transform should be moved to a class of its
own (and I also wonder whether the MultiDimensionalComplexMatrix name is
not misleading)
# FastFourierTransformer, FastSineTranformer and FastCosineTranformer
define public methods tranform2 and inversetransform2 but they are not
part of an interface. {color:red}As of {{r1213157}}, these methods have been
removed, and replaced by factory methods {{create()}} and {{createUnitary()}}
(FFT) or {{createOrthogonal()}} (FCT, FST).{color}
# Code uses variables that start with an uppercase. {color:red}Fixed,
together with various formatting issues.{color}
# FastHadamardTransformer contains illegible developer documentation (see
Javadoc for protected method fht). {color:red}Tried to improve things in
{{r1208986}}, but things are still a bit obscure. Besides, the link provided
is broken. Will look into that.{color}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-677) About package transform
[
https://issues.apache.org/jira/browse/MATH-677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13197502#comment-13197502
]
Sébastien Brisard commented on MATH-677:
All suggested changes but 3.1 and 3.3 have now been applied. I think this issue
can be resolved for the time being. Indeed, FFT implementation will probably
evolve in future versions of Commons-Math, especially for multidimensional
transforms. This should require thorough discussion.
About package transform
-
Key: MATH-677
URL: https://issues.apache.org/jira/browse/MATH-677
Project: Commons Math
Issue Type: Improvement
Reporter: Gilles
Assignee: Sébastien Brisard
Priority: Minor
Labels: api-change
Fix For: 3.0
Classes in package o.a.c.m.transform might require some changes in order to
conform to goals set for the next major release.
Some observations:
# Exceptions {color:red}done (see below){color}
## Should remove use of deprecated MathRuntimeException
## Should throw more specific Math...Exception instances instead of
standard IAE
# Interface RealTransformer (and implementations) contain non-conformant
method names (e.g. inversetransform instead of inverseTransform).
{color:red}Fixed in {{r1208293}}.{color}
# FastFourierTransformer:
## Methods mdfft and verifyDataSet take an argument of type Object (to
allow an argument with an unspecified number of dimensions)
## The RootsOfUnity helper class could be moved to the complex package.
{color:red}Done in {{r1238898}}{color}.
## For clarity, multidimensional transform should be moved to a class of its
own (and I also wonder whether the MultiDimensionalComplexMatrix name is
not misleading)
# FastFourierTransformer, FastSineTranformer and FastCosineTranformer
define public methods tranform2 and inversetransform2 but they are not
part of an interface. {color:red}As of {{r1213157}}, these methods have been
removed, and replaced by factory methods {{create()}} and {{createUnitary()}}
(FFT) or {{createOrthogonal()}} (FCT, FST).{color}
# Code uses variables that start with an uppercase. {color:red}Fixed,
together with various formatting issues.{color}
# FastHadamardTransformer contains illegible developer documentation (see
Javadoc for protected method fht). {color:red}Tried to improve things in
{{r1208986}}, but things are still a bit obscure. Besides, the link provided
is broken. Will look into that.{color}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-677) About package transform
[
https://issues.apache.org/jira/browse/MATH-677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13196749#comment-13196749
]
Sébastien Brisard commented on MATH-677:
h4. Refactoring of {{RootsOfUnity}}
In {{r1238179}}, the following changes have been adopted
* {{computeOmega(int n)}} now computes the roots of unity {{exp(i * k / n)}}.
{{FastFourierTransformer}} has been updated accordingly.
* {{isForward()}} has been renamed {{isCounterClockwise()}}, which refers to
the way the roots are ordered depending on the sign of {{n}}.
What do you think of these changes?
About package transform
-
Key: MATH-677
URL: https://issues.apache.org/jira/browse/MATH-677
Project: Commons Math
Issue Type: Improvement
Reporter: Gilles
Assignee: Sébastien Brisard
Priority: Minor
Labels: api-change
Fix For: 3.0
Classes in package o.a.c.m.transform might require some changes in order to
conform to goals set for the next major release.
Some observations:
# Exceptions {color:red}done (see below){color}
## Should remove use of deprecated MathRuntimeException
## Should throw more specific Math...Exception instances instead of
standard IAE
# Interface RealTransformer (and implementations) contain non-conformant
method names (e.g. inversetransform instead of inverseTransform).
{color:red}Fixed in {{r1208293}}.{color}
# FastFourierTransformer:
## Methods mdfft and verifyDataSet take an argument of type Object (to
allow an argument with an unspecified number of dimensions)
## The RootsOfUnity helper class could be moved to the complex package.
{color:red}Done in {{r1237544}}.{color} New class needs a little bit of
cleaning up (as well as unit tests), see below.
## For clarity, multidimensional transform should be moved to a class of its
own (and I also wonder whether the MultiDimensionalComplexMatrix name is
not misleading)
# FastFourierTransformer, FastSineTranformer and FastCosineTranformer
define public methods tranform2 and inversetransform2 but they are not
part of an interface. {color:red}As of {{r1213157}}, these methods have been
removed, and replaced by factory methods {{create()}} and {{createUnitary()}}
(FFT) or {{createOrthogonal()}} (FCT, FST).{color}
# Code uses variables that start with an uppercase. {color:red}Fixed,
together with various formatting issues.{color}
# FastHadamardTransformer contains illegible developer documentation (see
Javadoc for protected method fht). {color:red}Tried to improve things in
{{r1208986}}, but things are still a bit obscure. Besides, the link provided
is broken. Will look into that.{color}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-677) About package transform
[
https://issues.apache.org/jira/browse/MATH-677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13195965#comment-13195965
]
Sébastien Brisard commented on MATH-677:
h4. Refactoring of {{RootsOfUnity}}
{{RootsOfUnity}} is now exposed as a public class in package
{{o.a.c.m.complex}}. Therefore the connection with FFT is lost
* {{isForward()}} must be renamed
* {{computeOmega(int n)}} computes the roots of unity {{exp(-i * k / n)}}. I
think the minus sign is quite unexpected for a self-standing {{RootsOfUnity}}
class (in the FFT context, this minus sign does make perfect sense, of course).
So I propose to change this behavior (remove this minus sign).
About package transform
-
Key: MATH-677
URL: https://issues.apache.org/jira/browse/MATH-677
Project: Commons Math
Issue Type: Improvement
Reporter: Gilles
Assignee: Sébastien Brisard
Priority: Minor
Labels: api-change
Fix For: 3.0
Classes in package o.a.c.m.transform might require some changes in order to
conform to goals set for the next major release.
Some observations:
# Exceptions {color:red}done (see below){color}
## Should remove use of deprecated MathRuntimeException
## Should throw more specific Math...Exception instances instead of
standard IAE
# Interface RealTransformer (and implementations) contain non-conformant
method names (e.g. inversetransform instead of inverseTransform).
{color:red}Fixed in {{r1208293}}.{color}
# FastFourierTransformer:
## Methods mdfft and verifyDataSet take an argument of type Object (to
allow an argument with an unspecified number of dimensions)
## The RootsOfUnity helper class could be moved to the complex package.
{color:red}Done in {{r1237544}}.{color} New class needs a little bit of
cleaning up (as well as unit tests), see below.
## For clarity, multidimensional transform should be moved to a class of its
own (and I also wonder whether the MultiDimensionalComplexMatrix name is
not misleading)
# FastFourierTransformer, FastSineTranformer and FastCosineTranformer
define public methods tranform2 and inversetransform2 but they are not
part of an interface. {color:red}As of {{r1213157}}, these methods have been
removed, and replaced by factory methods {{create()}} and {{createUnitary()}}
(FFT) or {{createOrthogonal()}} (FCT, FST).{color}
# Code uses variables that start with an uppercase. {color:red}Fixed,
together with various formatting issues.{color}
# FastHadamardTransformer contains illegible developer documentation (see
Javadoc for protected method fht). {color:red}Tried to improve things in
{{r1208986}}, but things are still a bit obscure. Besides, the link provided
is broken. Will look into that.{color}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-677) About package transform
[
https://issues.apache.org/jira/browse/MATH-677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13190902#comment-13190902
]
Sébastien Brisard commented on MATH-677:
Used {{RealTransformAbstractTest}} for testing of {{FastSineTransformer}} (see
{{r1234685}}).
About package transform
-
Key: MATH-677
URL: https://issues.apache.org/jira/browse/MATH-677
Project: Commons Math
Issue Type: Improvement
Reporter: Gilles
Assignee: Sébastien Brisard
Priority: Minor
Labels: api-change
Fix For: 3.0
Classes in package o.a.c.m.transform might require some changes in order to
conform to goals set for the next major release.
Some observations:
# Exceptions
## Should remove use of deprecated MathRuntimeException
## Should throw more specific Math...Exception instances instead of
standard IAE
# Interface RealTransformer (and implementations) contain non-conformant
method names (e.g. inversetransform instead of inverseTransform).
{color:red}Fixed in {{r1208293}}.{color}
# FastFourierTransformer:
## Methods mdfft and verifyDataSet take an argument of type Object (to
allow an argument with an unspecified number of dimensions)
## The RootsOfUnity helper class could be moved to the complex package
## For clarity, multidimensional transform should be moved to a class of its
own (and I also wonder whether the MultiDimensionalComplexMatrix name is
not misleading)
# FastFourierTransformer, FastSineTranformer and FastCosineTranformer
define public methods tranform2 and inversetransform2 but they are not
part of an interface. {color:red}As of {{r1213157}}, these methods have been
removed, and replaced by factory methods {{create()}} and {{createUnitary()}}
(FFT) or {{createOrthogonal()}} (FCT, FST).{color}
# Code uses variables that start with an uppercase. {color:red}Fixed,
together with various formatting issues.{color}
# FastHadamardTransformer contains illegible developer documentation (see
Javadoc for protected method fht). {color:red}Tried to improve things in
{{r1208986}}, but things are still a bit obscure. Besides, the link provided
is broken. Will look into that.{color}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-677) About package transform
[
https://issues.apache.org/jira/browse/MATH-677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13190111#comment-13190111
]
Sébastien Brisard commented on MATH-677:
In {{r1234136}}, added a new
{{RealTransformAbstractTest}} allowing for automatic testing of
{{RealTransformers}}, with various data sizes (not only powers of two and the
likes). This will ease testing procedures if we ever implement transforms for
other types of data sizes.
About package transform
-
Key: MATH-677
URL: https://issues.apache.org/jira/browse/MATH-677
Project: Commons Math
Issue Type: Improvement
Reporter: Gilles
Assignee: Sébastien Brisard
Priority: Minor
Labels: api-change
Fix For: 3.0
Classes in package o.a.c.m.transform might require some changes in order to
conform to goals set for the next major release.
Some observations:
# Exceptions
## Should remove use of deprecated MathRuntimeException
## Should throw more specific Math...Exception instances instead of
standard IAE
# Interface RealTransformer (and implementations) contain non-conformant
method names (e.g. inversetransform instead of inverseTransform).
{color:red}Fixed in {{r1208293}}.{color}
# FastFourierTransformer:
## Methods mdfft and verifyDataSet take an argument of type Object (to
allow an argument with an unspecified number of dimensions)
## The RootsOfUnity helper class could be moved to the complex package
## For clarity, multidimensional transform should be moved to a class of its
own (and I also wonder whether the MultiDimensionalComplexMatrix name is
not misleading)
# FastFourierTransformer, FastSineTranformer and FastCosineTranformer
define public methods tranform2 and inversetransform2 but they are not
part of an interface. {color:red}As of {{r1213157}}, these methods have been
removed, and replaced by factory methods {{create()}} and {{createUnitary()}}
(FFT) or {{createOrthogonal()}} (FCT, FST).{color}
# Code uses variables that start with an uppercase. {color:red}Fixed,
together with various formatting issues.{color}
# FastHadamardTransformer contains illegible developer documentation (see
Javadoc for protected method fht). {color:red}Tried to improve things in
{{r1208986}}, but things are still a bit obscure. Besides, the link provided
is broken. Will look into that.{color}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-677) About package transform
[
https://issues.apache.org/jira/browse/MATH-677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13188315#comment-13188315
]
Sébastien Brisard commented on MATH-677:
Improved testing of {{FastFourierTransformer}} in {{r1232767}}.
About package transform
-
Key: MATH-677
URL: https://issues.apache.org/jira/browse/MATH-677
Project: Commons Math
Issue Type: Improvement
Reporter: Gilles
Assignee: Sébastien Brisard
Priority: Minor
Labels: api-change
Fix For: 3.0
Classes in package o.a.c.m.transform might require some changes in order to
conform to goals set for the next major release.
Some observations:
# Exceptions
## Should remove use of deprecated MathRuntimeException
## Should throw more specific Math...Exception instances instead of
standard IAE
# Interface RealTransformer (and implementations) contain non-conformant
method names (e.g. inversetransform instead of inverseTransform).
{color:red}Fixed in {{r1208293}}.{color}
# FastFourierTransformer:
## Methods mdfft and verifyDataSet take an argument of type Object (to
allow an argument with an unspecified number of dimensions)
## The RootsOfUnity helper class could be moved to the complex package
## For clarity, multidimensional transform should be moved to a class of its
own (and I also wonder whether the MultiDimensionalComplexMatrix name is
not misleading)
# FastFourierTransformer, FastSineTranformer and FastCosineTranformer
define public methods tranform2 and inversetransform2 but they are not
part of an interface. {color:red}As of {{r1213157}}, these methods have been
removed, and replaced by factory methods {{create()}} and {{createUnitary()}}
(FFT) or {{createOrthogonal()}} (FCT, FST).{color}
# Code uses variables that start with an uppercase. {color:red}Fixed,
together with various formatting issues.{color}
# FastHadamardTransformer contains illegible developer documentation (see
Javadoc for protected method fht). {color:red}Tried to improve things in
{{r1208986}}, but things are still a bit obscure. Besides, the link provided
is broken. Will look into that.{color}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-677) About package transform
[
https://issues.apache.org/jira/browse/MATH-677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13179252#comment-13179252
]
Sébastien Brisard commented on MATH-677:
Moved {{FastFourierTransformer.isPowerOf2(long)}} to
{{ArithmeticUtils.isPowerOfTwo(long)}} in {{r1227042}}.
About package transform
-
Key: MATH-677
URL: https://issues.apache.org/jira/browse/MATH-677
Project: Commons Math
Issue Type: Improvement
Reporter: Gilles
Assignee: Sébastien Brisard
Priority: Minor
Labels: api-change
Fix For: 3.0
Classes in package o.a.c.m.transform might require some changes in order to
conform to goals set for the next major release.
Some observations:
# Exceptions
## Should remove use of deprecated MathRuntimeException
## Should throw more specific Math...Exception instances instead of
standard IAE
# Interface RealTransformer (and implementations) contain non-conformant
method names (e.g. inversetransform instead of inverseTransform).
{color:red}Fixed in {{r1208293}}.{color}
# FastFourierTransformer:
## Methods mdfft and verifyDataSet take an argument of type Object (to
allow an argument with an unspecified number of dimensions)
## The RootsOfUnity helper class could be moved to the complex package
## For clarity, multidimensional transform should be moved to a class of its
own (and I also wonder whether the MultiDimensionalComplexMatrix name is
not misleading)
# FastFourierTransformer, FastSineTranformer and FastCosineTranformer
define public methods tranform2 and inversetransform2 but they are not
part of an interface. {color:red}As of {{r1213157}}, these methods have been
removed, and replaced by factory methods {{create()}} and {{createUnitary()}}
(FFT) or {{createOrthogonal()}} (FCT, FST).{color}
# Code uses variables that start with an uppercase. {color:red}Fixed,
together with various formatting issues.{color}
# FastHadamardTransformer contains illegible developer documentation (see
Javadoc for protected method fht). {color:red}Tried to improve things in
{{r1208986}}, but things are still a bit obscure. Besides, the link provided
is broken. Will look into that.{color}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-677) About package transform
[
https://issues.apache.org/jira/browse/MATH-677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13177966#comment-13177966
]
Sébastien Brisard commented on MATH-677:
h4. Exceptions
* remove references to {{MathRuntimeException}},
* throw more specific {{Math...Exception}} instances instead of standard
{{IAE}}.
As discussed on the
[mailing-list|http://mail-archives.apache.org/mod_mbox/commons-dev/201112.mbox/%3CCAGRH7Hq24t8O5hCPawVWFU1tOLAhcBrBjF%2BYh69zZivTnZRKFQ%40mail.gmail.com%3E],
{{MathIllegalArgumentException}} (as opposed to more specific subclasses) are
returned for non-power-of-two datasets.
(x) {{FastCosineTransformer}}
(x) {{FastFourierTransformer}}: done in {{r1226053}}
(x) {{FastHadamardTransformer}}
(x) {{FastSineTransformer}}
About package transform
-
Key: MATH-677
URL: https://issues.apache.org/jira/browse/MATH-677
Project: Commons Math
Issue Type: Improvement
Reporter: Gilles
Assignee: Sébastien Brisard
Priority: Minor
Labels: api-change
Fix For: 3.0
Classes in package o.a.c.m.transform might require some changes in order to
conform to goals set for the next major release.
Some observations:
# Exceptions
## Should remove use of deprecated MathRuntimeException
## Should throw more specific Math...Exception instances instead of
standard IAE
# Interface RealTransformer (and implementations) contain non-conformant
method names (e.g. inversetransform instead of inverseTransform).
{color:red}Fixed in {{r1208293}}.{color}
# FastFourierTransformer:
## Methods mdfft and verifyDataSet take an argument of type Object (to
allow an argument with an unspecified number of dimensions)
## The RootsOfUnity helper class could be moved to the complex package
## For clarity, multidimensional transform should be moved to a class of its
own (and I also wonder whether the MultiDimensionalComplexMatrix name is
not misleading)
# FastFourierTransformer, FastSineTranformer and FastCosineTranformer
define public methods tranform2 and inversetransform2 but they are not
part of an interface. {color:red}As of {{r1213157}}, these methods have been
removed, and replaced by factory methods {{create()}} and {{createUnitary()}}
(FFT) or {{createOrthogonal()}} (FCT, FST).{color}
# Code uses variables that start with an uppercase. {color:red}Fixed,
together with various formatting issues.{color}
# FastHadamardTransformer contains illegible developer documentation (see
Javadoc for protected method fht). {color:red}Tried to improve things in
{{r1208986}}, but things are still a bit obscure. Besides, the link provided
is broken. Will look into that.{color}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-677) About package transform
[
https://issues.apache.org/jira/browse/MATH-677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13167390#comment-13167390
]
Sébastien Brisard commented on MATH-677:
{{FastSineTransformer}} has been modified according to the above discussion in
revision {{1213157}}.
* {{transform2()}} and {{inverseTransform2()}} have gone,
* constructors have been made private,
* two factory methods are provided {{create()}} and {{createOrthogonal()}}.
About package transform
-
Key: MATH-677
URL: https://issues.apache.org/jira/browse/MATH-677
Project: Commons Math
Issue Type: Improvement
Reporter: Gilles
Assignee: Sébastien Brisard
Priority: Minor
Labels: api-change
Fix For: 3.0
Classes in package o.a.c.m.transform might require some changes in order to
conform to goals set for the next major release.
Some observations:
# Exceptions
## Should remove use of deprecated MathRuntimeException
## Should throw more specific Math...Exception instances instead of
standard IAE
# Interface RealTransformer (and implementations) contain non-conformant
method names (e.g. inversetransform instead of inverseTransform).
{color:red}Fixed in {{r1208293}}.{color}
# FastFourierTransformer:
## Methods mdfft and verifyDataSet take an argument of type Object (to
allow an argument with an unspecified number of dimensions)
## The RootsOfUnity helper class could be moved to the complex package
## For clarity, multidimensional transform should be moved to a class of its
own (and I also wonder whether the MultiDimensionalComplexMatrix name is
not misleading)
# FastFourierTransformer, FastSineTranformer and FastCosineTranformer
define public methods tranform2 and inversetransform2 but they are not
part of an interface
# Code uses variables that start with an uppercase. {color:red}Fixed,
together with various formatting issues.{color}
# FastHadamardTransformer contains illegible developer documentation (see
Javadoc for protected method fht). {color:red}Tried to improve things in
{{r1208986}}, but things are still a bit obscure. Besides, the link provided
is broken. Will look into that.{color}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-677) About package transform
[
https://issues.apache.org/jira/browse/MATH-677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13165887#comment-13165887
]
Sébastien Brisard commented on MATH-677:
{{FastCosineTransformer}} has been modified according to the above discussion
in revision {{1212262}}.
* {{transform2()}} and {{inverseTransform2()}} have gone,
* constructors have been made private,
* two factory methods are provided {{create()}} and {{createOrthogonal()}}.
About package transform
-
Key: MATH-677
URL: https://issues.apache.org/jira/browse/MATH-677
Project: Commons Math
Issue Type: Improvement
Reporter: Gilles
Assignee: Sébastien Brisard
Priority: Minor
Labels: api-change
Fix For: 3.0
Classes in package o.a.c.m.transform might require some changes in order to
conform to goals set for the next major release.
Some observations:
# Exceptions
## Should remove use of deprecated MathRuntimeException
## Should throw more specific Math...Exception instances instead of
standard IAE
# Interface RealTransformer (and implementations) contain non-conformant
method names (e.g. inversetransform instead of inverseTransform).
{color:red}Fixed in {{r1208293}}.{color}
# FastFourierTransformer:
## Methods mdfft and verifyDataSet take an argument of type Object (to
allow an argument with an unspecified number of dimensions)
## The RootsOfUnity helper class could be moved to the complex package
## For clarity, multidimensional transform should be moved to a class of its
own (and I also wonder whether the MultiDimensionalComplexMatrix name is
not misleading)
# FastFourierTransformer, FastSineTranformer and FastCosineTranformer
define public methods tranform2 and inversetransform2 but they are not
part of an interface
# Code uses variables that start with an uppercase. {color:red}Fixed,
together with various formatting issues.{color}
# FastHadamardTransformer contains illegible developer documentation (see
Javadoc for protected method fht). {color:red}Tried to improve things in
{{r1208986}}, but things are still a bit obscure. Besides, the link provided
is broken. Will look into that.{color}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-677) About package transform
[
https://issues.apache.org/jira/browse/MATH-677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13164212#comment-13164212
]
Sébastien Brisard commented on MATH-677:
{{FastFourierTransformer}} has been modified according to the above discussion
in revisions {{1211318}} and {{1211319}}.
* {{transform2()}} and {{inverseTransform2()}} have gone,
* constructors have been made private,
* two factory methods are provided {{create()}} and {{createUnitary}}.
This has a beneficial side effect: multi-dimensional FFT is no longer
restricted to the former {{transform2()}}/{{inverseTransform2()}} normalization
convention, as was previously the case.
About package transform
-
Key: MATH-677
URL: https://issues.apache.org/jira/browse/MATH-677
Project: Commons Math
Issue Type: Improvement
Reporter: Gilles
Assignee: Sébastien Brisard
Priority: Minor
Labels: api-change
Fix For: 3.0
Classes in package o.a.c.m.transform might require some changes in order to
conform to goals set for the next major release.
Some observations:
# Exceptions
## Should remove use of deprecated MathRuntimeException
## Should throw more specific Math...Exception instances instead of
standard IAE
# Interface RealTransformer (and implementations) contain non-conformant
method names (e.g. inversetransform instead of inverseTransform).
{color:red}Fixed in {{r1208293}}.{color}
# FastFourierTransformer:
## Methods mdfft and verifyDataSet take an argument of type Object (to
allow an argument with an unspecified number of dimensions)
## The RootsOfUnity helper class could be moved to the complex package
## For clarity, multidimensional transform should be moved to a class of its
own (and I also wonder whether the MultiDimensionalComplexMatrix name is
not misleading)
# FastFourierTransformer, FastSineTranformer and FastCosineTranformer
define public methods tranform2 and inversetransform2 but they are not
part of an interface
# Code uses variables that start with an uppercase. {color:red}Fixed,
together with various formatting issues.{color}
# FastHadamardTransformer contains illegible developer documentation (see
Javadoc for protected method fht). {color:red}Tried to improve things in
{{r1208986}}, but things are still a bit obscure. Besides, the link provided
is broken. Will look into that.{color}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-677) About package transform
[
https://issues.apache.org/jira/browse/MATH-677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13159964#comment-13159964
]
Gilles commented on MATH-677:
-
Thanks Sébastien for having a look at this.
As I never used these classes, I couldn't come up with concrete change
proposals regarding the API.
Rather than different constructors, we could have a new class. I recall that
the solvers package used to contain a single MullerSolver class with a
solve2 method (in addition to the solve defined in the interface). Now,
there is a MullerSolver2 that conforms to the interface.
If no-one objects, you could perform that change; but I'm afraid that a
thorough review of all those classes will have to wait for 4.0. The handling of
multiple dimension (through arguments of type Object) is particularly awkward
(for a strongly typed language like Java).
In addition to the observations indicated in the issue description, I also
notice a (private) class (MultiDimensionalComplexMatrix in
FastFourierTransformer) that implements the standard Cloneable interface.
This is best to be avoided (cf. Effective Java).
About package transform
-
Key: MATH-677
URL: https://issues.apache.org/jira/browse/MATH-677
Project: Commons Math
Issue Type: Improvement
Reporter: Gilles
Assignee: Sébastien Brisard
Priority: Minor
Labels: api-change
Fix For: 3.0
Classes in package o.a.c.m.transform might require some changes in order to
conform to goals set for the next major release.
Some observations:
# Exceptions
## Should remove use of deprecated MathRuntimeException
## Should throw more specific Math...Exception instances instead of
standard IAE
# Interface RealTransformer (and implementations) contain non-conformant
method names (e.g. inversetransform instead of inverseTransform).
{color:red}Fixed in {{r1208293}}.{color}
# FastFourierTransformer:
## Methods mdfft and verifyDataSet take an argument of type Object (to
allow an argument with an unspecified number of dimensions)
## The RootsOfUnity helper class could be moved to the complex package
## For clarity, multidimensional transform should be moved to a class of its
own (and I also wonder whether the MultiDimensionalComplexMatrix name is
not misleading)
# FastFourierTransformer, FastSineTranformer and FastCosineTranformer
define public methods tranform2 and inversetransform2 but they are not
part of an interface
# Code uses variables that start with an uppercase
# FastHadamardTransformer contains illegible developer documentation (see
Javadoc for protected method fht)
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-677) About package transform
[
https://issues.apache.org/jira/browse/MATH-677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13159988#comment-13159988
]
Sébastien Brisard commented on MATH-677:
Hi Gilles,
thanks for your advice. So I'll confine to the cosmetic changes, and we will
postpone the rest to the next major release. I'm a regular user of FFTs, and
I've been involved in debugging JTransforms, as well as setting up the whole
set of unit tests for this library. In short, I am quite willing to dive deeper
into this package... when we get some time.
Regarding point 4, I like your solution, because one class would correspond to
one unique pair {{transform}}/{{inverseTransform}}, with no risk to invoke
{{transform}} followed by {{inverseTransform2}} (and vice-versa).
However, if I may be the devil's advocate, I have a very small objection (but I
can live with that). {{transform}} and {{transform2}} actually perform the
*same* transform, only with a different prefactor. But the transform is
essentially the same (see the implementation).
On the other hand, there are other *types* of DCTs/DSTs on the market. The
currently implemented DCT is in fact DCT-I. It would be nice at some point to
have in CM DCT-II, DCT-III, ... which would be different classes. But then, we
would need classes for DCT-Ia, DCT-Ib, ..., DCT-IIa, DCT-IIb, ... for different
normalizing factors. It might be a bit complicated, and would put on the same
logical level *variants* of the same type of DCT, with different types of DCTs,
which I'm slightly (but only very slightly) uncomfortable with.
To sum up: the two versions of DCT currently implemented in the same class are
essentially the same up to a scaling factor. I dislike the existence of
{{transform2}}/{{inverseTransform2}}, but I think version 2 of the DCT should
be performed by the same class. That's the reason why I was proposing factory
methods (say {{createVersion1()}}/{{createVersion2()}}. Different classes would
be reserved to different versions of the DCT/DST.
Again, these thoughts are only for the sake of arguing, I'm quite happy with
any solution anyone would prefer.
Sébastien
About package transform
-
Key: MATH-677
URL: https://issues.apache.org/jira/browse/MATH-677
Project: Commons Math
Issue Type: Improvement
Reporter: Gilles
Assignee: Sébastien Brisard
Priority: Minor
Labels: api-change
Fix For: 3.0
Classes in package o.a.c.m.transform might require some changes in order to
conform to goals set for the next major release.
Some observations:
# Exceptions
## Should remove use of deprecated MathRuntimeException
## Should throw more specific Math...Exception instances instead of
standard IAE
# Interface RealTransformer (and implementations) contain non-conformant
method names (e.g. inversetransform instead of inverseTransform).
{color:red}Fixed in {{r1208293}}.{color}
# FastFourierTransformer:
## Methods mdfft and verifyDataSet take an argument of type Object (to
allow an argument with an unspecified number of dimensions)
## The RootsOfUnity helper class could be moved to the complex package
## For clarity, multidimensional transform should be moved to a class of its
own (and I also wonder whether the MultiDimensionalComplexMatrix name is
not misleading)
# FastFourierTransformer, FastSineTranformer and FastCosineTranformer
define public methods tranform2 and inversetransform2 but they are not
part of an interface
# Code uses variables that start with an uppercase
# FastHadamardTransformer contains illegible developer documentation (see
Javadoc for protected method fht)
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-677) About package transform
[
https://issues.apache.org/jira/browse/MATH-677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13159993#comment-13159993
]
Gilles commented on MATH-677:
-
By all means, if you have clear ideas about how to improve the package, I'm all
for it. If the changes can included in the upcoming release, even better.
If you have practical use-cases for FFT, then you are in the best position to
decide.
Factories may indeed be the way (although with better names than
createVersionX): I agree that a detail like a scaling factor might not
warrant a new class.
About package transform
-
Key: MATH-677
URL: https://issues.apache.org/jira/browse/MATH-677
Project: Commons Math
Issue Type: Improvement
Reporter: Gilles
Assignee: Sébastien Brisard
Priority: Minor
Labels: api-change
Fix For: 3.0
Classes in package o.a.c.m.transform might require some changes in order to
conform to goals set for the next major release.
Some observations:
# Exceptions
## Should remove use of deprecated MathRuntimeException
## Should throw more specific Math...Exception instances instead of
standard IAE
# Interface RealTransformer (and implementations) contain non-conformant
method names (e.g. inversetransform instead of inverseTransform).
{color:red}Fixed in {{r1208293}}.{color}
# FastFourierTransformer:
## Methods mdfft and verifyDataSet take an argument of type Object (to
allow an argument with an unspecified number of dimensions)
## The RootsOfUnity helper class could be moved to the complex package
## For clarity, multidimensional transform should be moved to a class of its
own (and I also wonder whether the MultiDimensionalComplexMatrix name is
not misleading)
# FastFourierTransformer, FastSineTranformer and FastCosineTranformer
define public methods tranform2 and inversetransform2 but they are not
part of an interface
# Code uses variables that start with an uppercase
# FastHadamardTransformer contains illegible developer documentation (see
Javadoc for protected method fht)
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-677) About package transform
[
https://issues.apache.org/jira/browse/MATH-677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13160709#comment-13160709
]
Sébastien Brisard commented on MATH-677:
{quote}
although with better names than createVersionX
{quote}
... goes without saying! :D
What do you think of
* {{create()}} for default normalization (corresponding to the current
{{transform()}}),
* {{createOrthogonal()}} for the other normalization (correspondind to the
current {{transform2()}}). From
[Wikipedia|http://en.wikipedia.org/wiki/Discrete_cosine_transform]
{quote}
This makes the DCT-I matrix orthogonal [...] but breaks the direct
correspondence with a real-even DFT.
{quote}
About package transform
-
Key: MATH-677
URL: https://issues.apache.org/jira/browse/MATH-677
Project: Commons Math
Issue Type: Improvement
Reporter: Gilles
Assignee: Sébastien Brisard
Priority: Minor
Labels: api-change
Fix For: 3.0
Classes in package o.a.c.m.transform might require some changes in order to
conform to goals set for the next major release.
Some observations:
# Exceptions
## Should remove use of deprecated MathRuntimeException
## Should throw more specific Math...Exception instances instead of
standard IAE
# Interface RealTransformer (and implementations) contain non-conformant
method names (e.g. inversetransform instead of inverseTransform).
{color:red}Fixed in {{r1208293}}.{color}
# FastFourierTransformer:
## Methods mdfft and verifyDataSet take an argument of type Object (to
allow an argument with an unspecified number of dimensions)
## The RootsOfUnity helper class could be moved to the complex package
## For clarity, multidimensional transform should be moved to a class of its
own (and I also wonder whether the MultiDimensionalComplexMatrix name is
not misleading)
# FastFourierTransformer, FastSineTranformer and FastCosineTranformer
define public methods tranform2 and inversetransform2 but they are not
part of an interface
# Code uses variables that start with an uppercase. {color:red}Fixed,
together with various formatting issues.{color}
# FastHadamardTransformer contains illegible developer documentation (see
Javadoc for protected method fht). {color:red}Tried to improve things in
{{r1208986}}, but things are still a bit obscure. Besides, the link provided
is broken. Will look into that.{color}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-677) About package transform
[
https://issues.apache.org/jira/browse/MATH-677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13159856#comment-13159856
]
Sébastien Brisard commented on MATH-677:
Regarding point 4., transform2 and inversetransform2 merely correspond to
different normalization conventions. Correct me if I'm wrong, the user is
unlikely to use mixed conventions in the *same* calculation (well, *I* never
do!), maybe we could use parameterized constructors to specify this
normalization convention. In which case, {{transform2}} and
{{inverseTransform2}} could go.
Otherwise, we would need an altogether new interface, because the Hadamard
transform does not offer such a choice.
This option would also offer the possibility to implement other types of
DCT/DST in the future. This is just a thought, though, because in this case,
maybe factory methods would be better.
About package transform
-
Key: MATH-677
URL: https://issues.apache.org/jira/browse/MATH-677
Project: Commons Math
Issue Type: Improvement
Reporter: Gilles
Priority: Minor
Labels: api-change
Fix For: 3.0
Classes in package o.a.c.m.transform might require some changes in order to
conform to goals set for the next major release.
Some observations:
# Exceptions
## Should remove use of deprecated MathRuntimeException
## Should throw more specific Math...Exception instances instead of
standard IAE
# Interface RealTransformer (and implementations) contain non-conformant
method names (e.g. inversetransform instead of inverseTransform)
# FastFourierTransformer:
## Methods mdfft and verifyDataSet take an argument of type Object (to
allow an argument with an unspecified number of dimensions)
## The RootsOfUnity helper class could be moved to the complex package
## For clarity, multidimensional transform should be moved to a class of its
own (and I also wonder whether the MultiDimensionalComplexMatrix name is
not misleading)
# FastFourierTransformer, FastSineTranformer and FastCosineTranformer
define public methods tranform2 and inversetransform2 but they are not
part of an interface
# Code uses variables that start with an uppercase
# FastHadamardTransformer contains illegible developer documentation (see
Javadoc for protected method fht)
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
