[ https://issues.apache.org/jira/browse/MATH-732?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13204338#comment-13204338 ]
Sébastien Brisard commented on MATH-732: ---------------------------------------- Thanks Kurt, I took item #2 and #3 into account in {{r1242230}}. As for item #1, here is what I've done # Created {{enum FastFourierTransformer.Normalization}} which can be {{STANDARD}} or {{UNITARY}}. # The type of normalization is then specified in the constructor of {{FastFourierTransformer}} as well as the static method {{transformInPlace}}. # Removed the factory methods {{create()}} and {{createUnitary()}}. I would like to suggest one last change: I do not like boolean parameters (in public methods), as they are difficult to interpret. I see two options # Define an enum {FORWARD, INVERSE}, and replace the boolean parameter in {{transformInPlace}} with this enum, # Create two methods {{transformInPlace}} and {{inverseTransformInPlace}}. This would be consistent with class methods. The first option would be my preferred option, but then I suppose we should also change the class methods, and replace {{transform}} and {{inverseTransform}} with one single method taking FORWARD or INVERSE as an argument. > Major Speed Improvement to 1D Discrete Fourier Transform (approximately 5x-9x > improvement). Preserved public API 100%. Removed unnecessary use of instance > variables and instance state. > ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- > > Key: MATH-732 > URL: https://issues.apache.org/jira/browse/MATH-732 > Project: Commons Math > Issue Type: Improvement > Affects Versions: 3.0 > Reporter: Kurt Ostfeld > Assignee: Sébastien Brisard > Labels: api, perfomance > Fix For: 3.0 > > Attachments: DFTPerformanceWithPatch.png, > DFTPerformanceWithoutPatch.png, FastFourierTransformer.java, > FastFourierTransformerTest.java, Main.java > > > I wrote my own Discrete Fourier Transform function in Java and ran some > benchmarks and found that it ran dramatically faster than the Apache library > implementation. This is a pretty straight forward implementation of the > standard Cooley Tukey algorithm that I read out of a textbook. This passes > all the Apache library test cases plus several I had written on my own. I > created a source code library patch that preserves the public API completely > while changing the internal implementation to achieve the performance > improvement. > In addition to the performance issue, I suggest that Discrete Fourier > Transform functionality be provided as stateless pure functions (in Java this > would be implemented with static methods) just like most other math > functions. As-is, the library requires the user to instantiate a Transformer > instance which maintains instance state, which is an unecessary complexity > for a pure math function. I held off on this change since it would change the > public API and affect existing code. However, I see similar backward > compatability breaking API changes are already in the FastFourierTransformer > class in the 3.0 code base. -- 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