Hi Alex and Giles, Thanks for the feedback.
1) Backward Compatibility and Complex Interface Yes. I understand the backward compatibility requirement and my goal is to be fully backward compatible. Fortunately, the existing Complex class has private constructors and so it is possible to refactor it as an interface. I was able to make the change along with a ComplexCartesianImpl class and run all unit tests successfully. I did not have to make any changes to unit tests. "mvn test" runs successfully on my local machine after the changes https://github.com/sumanth-rajkumar/commons-numbers/blob/sumanth-gsoc-22/commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/Complex.java https://github.com/sumanth-rajkumar/commons-numbers/blob/sumanth-gsoc-22/commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexCartesianImpl.java This I assume should meet the backward compatibility requirements? The proposed functional interface changes introduces a new interface ComplexNumber. I think we could reuse the refactored Complex interface instead of a new ComplexNumber interface and still maintain full backward compatibility. This would provide flexibility to older applications to work with new implementations of Complex such as MutableComplexImpl or ComplexPolarImpl or even ComplexStructImpl in the future whenever Java supports value types. Please let me know what you think. 2) ComplexFunction and ComplexResult functional interfaces Yes the generic <R> type introduces noise and can be solved as you suggested. I was also thinking about an alternative that avoids the ComplexResult and the generic type <R>. We could split complex unary operators into two primitive functions ( ToDoubleFunction<Complex>) one returning the real part of result and other for imaginary part interface ComplexFunction { ToDoubleFunction<Complex> getReal() ; ToDoubleFunction<Complex> getImaginary() ; } And for example the Conjugate implementation would look like this ComplexFunction conj = new ComplexFunction2() { @Override public ToDoubleFunction<Complex> getReal() { return complex -> complex.real(); } @Override public ToDoubleFunction<Complex> getImaginary() { return complex -> -complex.imag(); } }; }; And the functions used like below in Complex and ComplexList Complex { // default implementation are immutable always returning new instance to maintain b/w compatibility default Complex applyFunction(ComplexFunction function) { return Complex.ofCartesian(function.getReal().applyAsDouble(this),function.getImaginary().applyAsDouble(this)); } } } ComplexList { .. // applies changes in place void forEach(ComplexFunction fun) { ToDoubleFunction<Complex> realFunc = fun.getReal(); ToDoubleFunction<Complex> imgFunc = fun.getImaginary(); ComplexCursor cursor = new ComplexCursor(); while (cursor.index < r.length) { cursor.apply(realFunc.applyAsDouble(cursor), imgFunc .applyAsDouble(cursor)); cursor.index++; } } ... } Does this make sense or we just stick to the original interface that includes ComplexResult<R>? 3) Naming convention for the Functional Interfaces On reviewing the functions in java.util.functions package, the convention is "Function" name is used for interfaces that can accept inputs of different types and return result of different type "Operator" are specialization of "Function" that take same type for all inputs and result Should we follow a similar naming convention for Complex functional interfaces? Thanks On Fri, 27 May 2022 at 21:34, Alex Herbert <alex.d.herb...@gmail.com> wrote: > On Thu, 26 May 2022 at 15:04, Gilles Sadowski <gillese...@gmail.com> > wrote: > > > > > > Next, I wanted to mention how I plan to start this project and was > hoping > > > to get some feedback. > > > > > > As per my proposal, the first thing I wanted to start with was the API > > > design which would have interfaces to represent complex numbers, > methods > > to > > > convert to/from linear primitive arrays, Java 8 functional interfaces > for > > > unary/binary operators and for functions for complex operations and > > > transforms involving: complex number and real numbers, complex vectors > > and > > > scalars, complex matrix, vectors and scalars. > > > > There are many items here. I would suggest breaking it down. Perhaps: > > 1. > interfaces to represent complex numbers > unary/binary operators and for functions for complex operations > > 2. > methods to convert to/from linear primitive arrays > > 3. > complex vectors and scalars, complex matrix, vectors and scalars. > > > Although not completely independant, we could discuss each in turn and see > what functionality is required. > > I will start with topic 1. Currently we have a single object, Complex, that > represents a complex number in cartesian form. It has a full set of > operations specified in ISO C99. I would suggest you have a look at the > specification as it has a lot of information about this [1]. > > There is a benchmark for these operations in the examples JMH module: > org.apache.commons.numbers.examples.jmh.complex.ComplexPerformance > > Ideally any changes to extract all the methods into a static class should > not impact performance. Many of the methods are quite involved and > therefore slow. However some methods such as those for > add/subtract/multiply/divide with real or imaginary scalars will be fast. > It would be interesting to see if abstraction to a static class impacts > their performance. These operations are not in the JMH benchmark so this > could be added to provide a reference point for these. > > > From your GH code you have the following interface: > > public interface ComplexFunction { > <R> R apply(double r, double i, ComplexResult<R> result); > } > > I cannot create a lambda function for this as the method has a generic type > parameter. This fails to compile. > > ComplexFunction f = (r, i, result) -> { > // conjugate > return result.apply(r, -i); > }; > > This can be solved by moving <R> to the interface declaration: > > public interface ComplexFunction<R> { > R apply(double r, double i, ComplexResult<R> result); > } > > But then all use of ComplexFunction has to be typed which can get noisy. It > is however explicit in what the function output will be (and we assume the > input is a complex number of some sort). > > > Q. Do we wish to support effectively duplication of operations by accepting > primitives and also a ComplexNumber type in the static methods: > > interface ComplexNumber { > double real(); > double imag(); > } > > class ComplexFunctions { > <R extends ComplexNumber> R sin(ComplexNumber c, ComplexResult<R> r) { > return sin(c.real(), c.imag(), r); > } > <R extends ComplexNumber> R sin(double r, double i, ComplexResult<R>) { > // ... > } > } > > There are various options for chaining methods together for sequential > operations on the same complex. Should this avoid repeat object allocation > by providing a MutableComplex holder: > > class MutableComplex implements ComplexNumber, > ComplexResult<MutableComplex> { > // allows read, write from/to real, imaginary parts > } > > > > Q. How to manipulate ComplexList: > > class ComplexList implements List<Complex> { > private double[] r; > private double[] i; > } > > You have forEach methods (note this is without the type parameter): > > void forEach(ComplexFunction fun); > > So how do I declare a function to pass to the list, that accepts the real > and imaginary parts and saves them back to the list. Currently you have the > ComplexFunction accept the real and imaginary parts. But what if it > accepted a ComplexNumber: > > public interface ComplexFunction<R> { > R apply(ComplexNumber c, ComplexResult<R> result); > } > > > The ComplexList need only provide a single object that acts as a read/write > cursor over the data by implementing both interfaces ComplexNumber and > ComplexResult: > > // Internal class > class ComplexCursor implements ComplexNumber, ComplexResult<Void> { > // Directly manipulated by the enclosing list > int index; > > @Override > public Void apply(double r, double i) { > ComplexList.this.r[index] = r; > ComplexList.this.i[index] = i; > return null; > } > > @Override > public double real() { > return ComplexList.this.r[index]; > } > > @Override > public double imag() { > return ComplexList.this.i[index]; > } > } > > I can write the method: > > void forEach(ComplexFunction<Void> fun) { > ComplexCursor cursor = new ComplexCursor(); > while (cursor.index < r.length) { > fun.apply(cursor, cursor); > cursor.index++; > } > } > > And call it like this: > > class ComplexFunctions { > static <R> R conj(ComplexNumber c, ComplexResult<R> r) { > return r.apply(c.real(), -c.imag()); > } > } > > > ComplexList l; > l.forEach(ComplexFunctions::conj); > > // Using a lambda > l.forEach((c, result) -> { > return result.apply(c.real(), -c.imag()); > }); > > > I've provided a few ideas there to get started. In summary here are some > requirements we should keep in mind: > > - Complex functions should be able to be declared with lambda functions > - The ComplexList allows read/write type operations on elements with > complex functions > > Alex > > > [1] http://www.open-std.org/JTC1/SC22/WG14/www/standards, specifically > WG14 > N1256 sections 7.3 (p 170) and Annex G (p 467). >
