On Sun, 05 Jul 2015 17:34:46 -0700, Phil Steitz wrote:
On 7/5/15 11:31 AM, Luc Maisonobe wrote:
Hi all,
Le 05/07/2015 19:12, Gilles a écrit :
Hi.
On Sun, 05 Jul 2015 07:20:30 -0700, Phil Steitz wrote:
On 7/5/15 12:44 AM, Luc Maisonobe wrote:
Hi all,
A few days ago, several pull requests arrived from github
(<https://github.com/apache/commons-math/pull/6>,
<https://github.com/apache/commons-math/pull/7>,
<https://github.com/apache/commons-math/pull/8> and
<https://github.com/apache/commons-math/pull/9>). They
concern ebe-like functions for MathArrays.
I was ready to commit them, but have second thoughts now.
Wouldn't it
be better to have rather an API like:
MathArrays.map(UnivariateFunction f, double[] a)
and
MathArrays.map(BivariateFunction f, double[] a, double[] b)
It would avoid adding a bunch of functions.
There were already concerns about inflating API in RealVector
a few years ago and I would prefer not doing again the same
errors.
We might be doing a different kind of design mistake here: a low level
package ("o.a.c.m.util") now depends on higher level one
("o.a.c.m.analysis").
I agree with Phil in that "MathArrays" should only contain things that
are
used within CM. They are there primarily to avoid code duplication,
not as
syntactic sugar for users.
User-level utilities should go somewhere else.
In particular CM offers "ArrayRealVector" as an abstraction of an array
of
"double" values for users who want more functionality than the "raw"
Java
array offers.
As I indicated previously, "map" is there already, rightfully.
People who want to benefit from extra functionality should do:
double[] out = new ArrayRealVector(new double[] { ... }, false).map(new
UnivariateFunction() { ... }).toArray();
We should not duplicate the "map" functionality in "MathArrays".
For the "Bivariate" variant, we could consider adding it in
"ArrayRealVector".
Regards,
Gilles
What do you think?
Two quick comments:
1. +1 for general weight control. MathArrays has a big obesity
risk. We used to try to limit it to things that are actually
needed
inside [math] and sometimes I think it would be best to go back to
that rule.
+1 to the principle that methods in "MathArrays" should
1. be confined to low-level utilities, and
2. not imply that Java arrays are Cartesian coordinates.
So I guess the three methods
double[] map(final UnivariateFunction f, final double[] a)
double[] map(final BivariateFunction f,
final double[] a, final double[] b)
double[] map(final BivariateFunction f,
final double[] a, final double b)
are OK. I'll push my commit in a few minutes. It it doesn't fit
well, we can remove them afterwards.
+1 for CTR :)
Changes look fine to me.
Even the "cosAngle" which I recently proposed should not be
included
(if we go that route), and existing such methods (like "distance")
should be deprecated in favour of an abstraction that specifically
represents such a geometric quantity.
Use of more robust code could thus be enforced (cf. method "angle"
in class "o.a.c.m.geometry.euclidean.threed.Vector3D").
Also, isn't it a problem that we have a "general array" in
"RealVector"
(where the "map" method exists already) while that class also
implicitly
assumes that its elements are Cartesian coordinates?
I think that we should review and correct this design flaw (there
is
a very old issue about that IIRC).
A class "VectorND" (of arbitrary but fixed dimension) could be
created
in "o.a.c.m.geometry" and geometrical functionalities such as
"distance"
be defined there rather than in "RealVector" and "ArrayRealVector"
(that
are mutable) and/or in "MathArrays".
[One potential problem is the performance issue (memory/speed): We
should
have benchmarks that answer the question of whether using
"VectorND" is
the "same" as using a Java raw array.]
2. I can't find it right now, but I vaguely recall earlier
discussion on the smelliness of the ebe methods. Might be worth
looking through the archives.
Yes, this is what I did. There was a thread started about sparse
vectors
in June 2012 <http://commons.markmail.org/thread/linqfc5s2qugz3fp>
and
another message in another thred in August 2011
<http://markmail.org/message/67qdrfeho3reiihj>.
Thanks, that's what I was remembering.
IIRC, it was also an argument in the discussions about cleaning up
"RealVector" and "RealMatrix"?
Yes. The threads above were really related to linear algebra, not
MathArrays (which is a lower level IMHO).
What was said at that time is still true, and 4.0 is probably a good
opportunity to change this behaviour (I like Ted Dunning suggestion
in
the 2011 thread about views). However, as always it needs times and
it
seems none of us has time available now.
+1 - patches welcome!
Phil
best regards,
Luc
"ebe" is low level, so it's OK to have them in "MathArrays" (if
they
serve internally). But they also exist in "RealVector" where it
might,
or might not, be OK depending on the concept defined by such a
class
(which is now a hybrid of "general array" and "Cartesian
coordinates").
Regards,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
