[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[ https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17378451#comment-17378451 ] Matt Juntunen commented on NUMBERS-77: -- Closing this issue as no more utilities need to be moved into numbers. > Move utilities from "Commons Geometry" > -- > > Key: NUMBERS-77 > URL: https://issues.apache.org/jira/browse/NUMBERS-77 > Project: Commons Numbers > Issue Type: Task >Reporter: Gilles Sadowski >Priority: Major > Fix For: 1.1 > > Attachments: NUMBERS-77.diff > > Time Spent: 1h 40m > Remaining Estimate: 0h > > "Commons Geometry" defines utilities that would be a better fit in this > component. > Duplication of general-purpose codes should be avoided, in order to benefit > from consolidated usage (bug reporting, performance enhancements, ...). -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17362902#comment-17362902
]
Matt Juntunen commented on NUMBERS-77:
--
Created TEXT-207 for moving {{DoubleFormats}} to commons-text.
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Attachments: NUMBERS-77.diff
>
> Time Spent: 1h 40m
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[ https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17360277#comment-17360277 ] Matt Juntunen commented on NUMBERS-77: -- I sent an email to the dev list but received no interest, only a note that the JDK already contains number formatting capabilities. I'm currently working on simplifying the code in question and hopefully increasing the performance. Once that's done, I was planning on sending another email about it with some more specific use cases and performance benchmarks. > Move utilities from "Commons Geometry" > -- > > Key: NUMBERS-77 > URL: https://issues.apache.org/jira/browse/NUMBERS-77 > Project: Commons Numbers > Issue Type: Task >Reporter: Gilles Sadowski >Priority: Major > Fix For: 1.1 > > Attachments: NUMBERS-77.diff > > Time Spent: 1h 40m > Remaining Estimate: 0h > > "Commons Geometry" defines utilities that would be a better fit in this > component. > Duplication of general-purpose codes should be avoided, in order to benefit > from consolidated usage (bug reporting, performance enhancements, ...). -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[ https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17360274#comment-17360274 ] Gilles Sadowski commented on NUMBERS-77: Was there some discussion/conclusion about the latter suggestion? > Move utilities from "Commons Geometry" > -- > > Key: NUMBERS-77 > URL: https://issues.apache.org/jira/browse/NUMBERS-77 > Project: Commons Numbers > Issue Type: Task >Reporter: Gilles Sadowski >Priority: Major > Fix For: 1.1 > > Attachments: NUMBERS-77.diff > > Time Spent: 1h 40m > Remaining Estimate: 0h > > "Commons Geometry" defines utilities that would be a better fit in this > component. > Duplication of general-purpose codes should be avoided, in order to benefit > from consolidated usage (bug reporting, performance enhancements, ...). -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17334870#comment-17334870
]
Gilles Sadowski commented on NUMBERS-77:
bq. {{DoubleFunctionXN}}
It's unlikely that we'll promote these interfaces to common use. What is the
use-case outside of [Geometry]?
bq. {{SimpleTupleParser}}
{{SimpleTupleFormat}} ?
Same issue as above: [Geometry] *must* use that internally only. And it's
unlikely that we can promote it over parser generators...
bq. [...] a {{commons-numbers-text}} module?
Ouch; let's not (re)open that can of worms.
However, it seems that many things from {{o.a.c.geometry.io.core.utils}} could
find a home in ["Commons Text"|http://commons.apache.org/proper/commons-text/].
It would make sense that the "io" module implements its parsing of text-based
geometry formats using primitives defined in [Text].
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Attachments: NUMBERS-77.diff
>
> Time Spent: 1.5h
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17334766#comment-17334766
]
Matt Juntunen commented on NUMBERS-77:
--
Some additional types/utilities to consider moving:
* {{o.a.c.geometry.core.internal.DoubleFunctionXN}}
* {{o.a.c.geometry.core.internal.SimpleTupleParser}}
* {{o.a.c.geometry.io.core.utils.DoubleFormat}},
{{o.a.c.geometry.io.core.utils.DoubleFormats}}, and the package-private
{{o.a.c.geometry.io.core.utils.ParsedDouble}}
Perhaps the last two could go in a {{commons-numbers-text}} module?
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Attachments: NUMBERS-77.diff
>
> Time Spent: 1.5h
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17334750#comment-17334750
]
Matt Juntunen commented on NUMBERS-77:
--
bq. We should add unit tests.
Added in PR https://github.com/apache/commons-numbers/pull/91. Some of the
relevant tests in geometry were in in the test for the abstract
{{DoublePrecisionContext}} class. I just moved those over as well.
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Attachments: NUMBERS-77.diff
>
> Time Spent: 1.5h
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[ https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17329907#comment-17329907 ] Gilles Sadowski commented on NUMBERS-77: We should [add unit tests|https://coveralls.io/builds/39054112/source?filename=commons-numbers-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fcommons%2Fnumbers%2Fcore%2FPrecision.java]. > Move utilities from "Commons Geometry" > -- > > Key: NUMBERS-77 > URL: https://issues.apache.org/jira/browse/NUMBERS-77 > Project: Commons Numbers > Issue Type: Task >Reporter: Gilles Sadowski >Priority: Major > Fix For: 1.1 > > Attachments: NUMBERS-77.diff > > Time Spent: 1h 10m > Remaining Estimate: 0h > > "Commons Geometry" defines utilities that would be a better fit in this > component. > Duplication of general-purpose codes should be avoided, in order to benefit > from consolidated usage (bug reporting, performance enhancements, ...). -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17329898#comment-17329898
]
Gilles Sadowski commented on NUMBERS-77:
I've modified {{Precision.compareTo}} according to Alex's suggested fix (cf.
NUMBERS-154).
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Attachments: NUMBERS-77.diff
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[ https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17329414#comment-17329414 ] Alex Herbert commented on NUMBERS-77: - Snapshots are deployed by Jenkins: [commons-numbers-core|https://repository.apache.org/content/repositories/snapshots/org/apache/commons/commons-numbers-core/] > Move utilities from "Commons Geometry" > -- > > Key: NUMBERS-77 > URL: https://issues.apache.org/jira/browse/NUMBERS-77 > Project: Commons Numbers > Issue Type: Task >Reporter: Gilles Sadowski >Priority: Major > Fix For: 1.1 > > Attachments: NUMBERS-77.diff > > Time Spent: 1h 10m > Remaining Estimate: 0h > > "Commons Geometry" defines utilities that would be a better fit in this > component. > Duplication of general-purpose codes should be avoided, in order to benefit > from consolidated usage (bug reporting, performance enhancements, ...). -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17329409#comment-17329409
]
Alex Herbert commented on NUMBERS-77:
-
{quote}Let's refer to what's in "master" (there were fixes).
{quote}
Master was updated as I was writing the post. The javadoc looks fine.
Here is some more thoughts on compare and the Comparator interface.
I think that we can handle NaN by specifying that the compare returns [-1, 0,
1] for an ordering of non Nan numbers with equality (a zero result) defined by
some measure of closeness. NaN inputs are handled using Double.compare. So two
input NaNs will return 0:
{code:java}
public static int compareTo(double x, double y, double eps) {
if (equals(x, y, eps)) {
return 0;
} else if (x < y) {
return -1;
} else if (x > y) {
return 1;
}
// NaN input
return Double.compare(x, y);
}
{code}
Using this creates a function that handles non-finite numbers as per
Double.compare and can sort an array but not necessarily in its strict natural
order. The sort function in Arrays is described as stable, meaning that two
elements that have a compare result of 0 will not be reordered. Thus if you
resort the array you get the same result.
Sorting Matt's example using Precision as is:
{code:java}
@Test
void testUnstableSort() {
Double[] array = {0.02, 0.01, 1.0, 2.0, Double.NaN, Double.NaN};
for (int i = 0; i < 10; i++) {
Collections.shuffle(Arrays.asList(array));
Arrays.sort(array, (a, b) -> Precision.compareTo(a, b, 0.1));
System.out.println(Arrays.toString(array));
}
}
{code}
{noformat}
[NaN, 0.02, 0.01, 1.0, 2.0, NaN]
[0.01, 2.0, NaN, 0.02, 1.0, NaN]
[0.02, NaN, 0.01, 1.0, 2.0, NaN]
[NaN, NaN, 0.01, 0.02, 1.0, 2.0]
[NaN, 0.02, 0.01, 1.0, 2.0, NaN]
[0.02, 0.01, 1.0, 2.0, NaN, NaN]
[NaN, 0.02, 0.01, 1.0, NaN, 2.0]
[1.0, NaN, 0.01, 0.02, 2.0, NaN]
[0.01, 0.02, 1.0, 2.0, NaN, NaN]
[1.0, NaN, NaN, 0.02, 0.01, 2.0]
{noformat}
With the change to the above code:
{noformat}
[0.01, 0.02, 1.0, 2.0, NaN, NaN]
[0.01, 0.02, 1.0, 2.0, NaN, NaN]
[0.01, 0.02, 1.0, 2.0, NaN, NaN]
[0.02, 0.01, 1.0, 2.0, NaN, NaN]
[0.02, 0.01, 1.0, 2.0, NaN, NaN]
[0.02, 0.01, 1.0, 2.0, NaN, NaN]
[0.02, 0.01, 1.0, 2.0, NaN, NaN]
[0.01, 0.02, 1.0, 2.0, NaN, NaN]
[0.02, 0.01, 1.0, 2.0, NaN, NaN]
[0.02, 0.01, 1.0, 2.0, NaN, NaN]
{noformat}
Looking at the javadoc for Comparator:
# sgn(compare(x, y)) == -sgn(compare(y, x)) for all x and y
# relation is transitive: (compare(x, y) > 0) & (compare(y, z) > 0))
implies compare(x, z) > 0
# compare(x, y)==0 implies that sgn(compare(x, z))==sgn(compare(y, z)) for all
z
# not strictly required that (compare(x, y)==0) == (x.equals( y )). Generally
speaking, any comparator that violates this condition should clearly indicate
this fact
I think that point 1 and 2 are covered. Point 4 is not but that is not
required.
But point 3 is not. Consider the points x > y > z:
{noformat}
Epsilon: |-|
|-|-|
x y z
compare(x, y) == 0
compare(x, z) == 1
compare(y, z) == 0 (violation, it should be 1){noformat}
Here x is equal to y but above z. So y should be above z but it is not.
Thus we cannot implement the contract of a comparator. This means we do not
have to change anything and can handle NaN in whatever way we like during
comparison.
Some options are:
# Leave the method and document it as unsuitable for use as part of the
implementation for Comparator.
# Update it to handle NaN as per Double.compare. It is still not strictly
suitable as part of the implementation for Comparator due to violation of rule
3. It works better for sorting but would return 0 when Precision.equals(NaN,
NaN) returns false.
# Remove the compare method from Precision, i.e. what is the use case if not
for ordering.
# Others...
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Attachments: NUMBERS-77.diff
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[ https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17329376#comment-17329376 ] Matt Juntunen commented on NUMBERS-77: -- Still not sold on the name but I'm 99% sure this will work for geometry. On a practical note, can I use this now or do I need to wait until it's released? Can Jenkins and Travis resolve a SNAPSHOT dependency on commons-numbers? > Move utilities from "Commons Geometry" > -- > > Key: NUMBERS-77 > URL: https://issues.apache.org/jira/browse/NUMBERS-77 > Project: Commons Numbers > Issue Type: Task >Reporter: Gilles Sadowski >Priority: Major > Fix For: 1.1 > > Attachments: NUMBERS-77.diff > > Time Spent: 1h 10m > Remaining Estimate: 0h > > "Commons Geometry" defines utilities that would be a better fit in this > component. > Duplication of general-purpose codes should be avoided, in order to benefit > from consolidated usage (bug reporting, performance enhancements, ...). -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17329363#comment-17329363
]
Gilles Sadowski commented on NUMBERS-77:
bq. In your diff [...]
Let's refer to what's in "master" (there were fixes).
bq. comparisons break down when NaN is input. Is this the behaviour we want?
Probably not if it can be helped. ;-)
bq. I like having the term "precision" in the name [...]
Then why
bq. prefer if the interface were top-level
since the nesting offers both?
bq. I'm going to need to update about a bajillion variable names [...]
Why?
It's perfectly fine that
{code}
final DoublePrecisionContext precision = new
EpsilonDoublePrecisionContext(1e-4);
{code}
becomes
{code}
final Precision.DoubleEquivalence precision =
Precision.doubleEquivalenceOfEpsilon(1e-4);
{code}
bq. [...] values [0.01, 0.02, 1, 2] are "correctly" sorted when using an
epsilon value of 0.1 but so are [0.02, 0.01, 1, 2].
Not a problem since indeed 0.01 and 0.02 are _equivalent_. Hence the name. ;-)
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Attachments: NUMBERS-77.diff
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17329345#comment-17329345
]
Matt Juntunen commented on NUMBERS-77:
--
{quote}So currently the comparisons break down when NaN is input. Is this the
behaviour we want?
{quote}
It would be better if we could provide a comparison that produces a stable
sort. However, this may not be possible based on our use of "fuzzy"
comparisons. For example, the values {{[0.01, 0.02, 1, 2]}} are "correctly"
sorted when using an epsilon value of {{0.1}} but so are {{[0.02, 0.01, 1,
2]}}. The first two values are considered equal.
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Attachments: NUMBERS-77.diff
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17329342#comment-17329342
]
Matt Juntunen commented on NUMBERS-77:
--
bq. So if we agree on the basic design, I could merge the current version (as
per the attached diff). OK?
Yes, but I have a couple more points:
- I'm not sold on the name yet (see below).
- I would prefer if the interface were top-level instead of nested in
{{Precision}}. It makes it more convenient since this type will be used almost
everywhere in geometry.
bq. I feel that it doesn't say what the class does.
It's the same with the {{Precision}} class in general. I like having the term
"precision" in the name since you immediately know that you're going to be
working with floating point accuracy. My favorite names so far are (in order):
- {{PrecisionContext}}
- {{PrecisionComparator}}
- {{PrecisionCompareStrategy}}
(I'm probably a bit biased here, though, since if the name doesn't have
"precision" in it, I'm going to need to update about a bajillion variable names
and javadocs in geometry :-)
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Attachments: NUMBERS-77.diff
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17329332#comment-17329332
]
Alex Herbert commented on NUMBERS-77:
-
{quote}I think that there is a problem when {{0 < |a| < eps}}
{quote}
Good spot. It will return a and not 0.
Looking at Matt's example in AbstractHyperPlane then this is a case where -0.0
should not return -1 if you test it with 0.0. But that should be handled by an
epsilon for the eqZero call.
In your diff for the sign method your javadoc states:
{noformat}
* The returned value is
*
* {@code 0} if the arguments are considered equal,
* {@code -1} if {@code a < b},
* {@code +1} if {@code a > b} or if either value is NaN.
* {noformat}
This is copied from compare. It should be:
{noformat}
* The returned value is
*
* {@code 0} if the arguments is considered equal to zero,
* {@code -1} if {@code a < 0},
* {@code +1} if {@code a > 0} or if either value is NaN.
* {noformat}
Thinking about the result for compare, it is documented to return +1 when
either argument is NaN. If this class is ever used for sorting (as part of a
comparison of objects with double values) then this will result in an unstable
sort as this violates the expected behaviour:
{noformat}
if compare(a, NaN) == 1 then compare(NaN, a) == -1
{noformat}
Double.compare will place NaN after any non-Nan value, or any non-NaN value
before a NaN. It returns 0 for two NaN inputs.
{noformat}
compare(a, NaN) == -1
compare(NaN, a) == 1
compare(NaN, NaN) == 0
{noformat}
I think that NaN requires some special handling. For example assuming compare
returns 1 for NaN on either input you have:
{noformat}
eq(a, NaN) == false
eqZero(NaN) == false
lt(a, NaN) == false
lt(NaN, a) == false
lte(a, NaN) == false
lte(NaN, a) == false
gt(a, NaN) == true
gt(NaN, a) == true
gte(a, NaN) == true
gte(NaN, a) == true
sign(NaN) == 1{noformat}
This issue is also present in the Precision class which will return +1 for the
compare methods when NaN is one of the arguments. All the Precision methods for
equality state that nothing is equal to NaN and there are separate methods for
equalsIncludingNan. But the compare method does not specially handle it and so
cannot be used for sorting/ordering.
If you have an object that implements equals and compare using Precision.equals
and Precision.compareTo then it cannot be used in a sort as comparisons with
NaN always return 1. If you use Precision.equalsIncludingNan then the
comparator will fail its contract as two equal objects (NaN, NaN) will not have
zero returned from compare(NaN, NaN).
So currently the comparisons break down when NaN is input. Is this the
behaviour we want? If so it should be documented to not use these methods in
comparators for sorting.
Note you can test this using:
{code:java}
@Test
void testUnstableSort() {
Double[] array = {1.0, 2.0, 3.0, Double.NaN, Double.NaN};
for (int i = 0; i < 10; i++) {
Collections.shuffle(Arrays.asList(array));
Arrays.sort(array, (a, b) -> Precision.compareTo(a, b, 1e-10));
// CHECKSTYLE stop all
System.out.println(Arrays.toString(array));
}
}
{code}
Outputs some rubbish:
{noformat}
[NaN, 1.0, 2.0, 3.0, NaN]
[2.0, NaN, 1.0, NaN, 3.0]
[1.0, 2.0, 3.0, NaN, NaN]
[3.0, NaN, NaN, 1.0, 2.0]
[3.0, NaN, 1.0, 2.0, NaN]
[2.0, NaN, NaN, 1.0, 3.0]
[1.0, NaN, NaN, 2.0, 3.0]
[3.0, NaN, 1.0, NaN, 2.0]
[1.0, NaN, NaN, 2.0, 3.0]
[3.0, NaN, 1.0, 2.0, NaN]
{noformat}
Switching to Double.compare and it works.
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Attachments: NUMBERS-77.diff
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17329318#comment-17329318
]
Gilles Sadowski commented on NUMBERS-77:
Merged (with discussed changes) into master:
commit dbd2a473e4949d895054190f61cb950da1d6b36d
Name of the interface still TBC, as well as the implementation of {{signum}}.
Please check.
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Attachments: NUMBERS-77.diff
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17329237#comment-17329237
]
Gilles Sadowski commented on NUMBERS-77:
{quote}
{code}
default double signum(double a) {
return (a == 0.0 || compare(a, 0.0) == 0 || Double.isNaN(a)) ? a :
Math.copySign(1.0, a);
}
{code}
{quote}
I think that there is a problem when {{0 < |a| < eps}}.
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Attachments: NUMBERS-77.diff
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17329202#comment-17329202
]
Gilles Sadowski commented on NUMBERS-77:
bq. [interface with default methods] is much cleaner.
So if we agree on the basic design, I could merge the current version (as per
the attached diff). OK?
bq. Are we sure that PrecisionContext isn't the best option?
I feel that it doesn't say *what* the class does.
bq. I was picturing this as an easy way to compare to zero. [...] handling of
-0.0 [...] is entirely up to the implementation.
It's potentially confusing (giving the false sense that it can be used wherever
"signum" would be).
bq. It's essentially a signum but returning an int for strict equality checking
(eg, cmp == 0).
It can be done the same with the return type being {{double}} (and the
appropriate comment that a strict equality check is done on purpose).
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Attachments: NUMBERS-77.diff
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17329187#comment-17329187
]
Matt Juntunen commented on NUMBERS-77:
--
bq. Could the functionality be used as is in [Geometry]?
Yes, I believe so.
bq. Maybe the name is still not right.
Are we sure that {{PrecisionContext}} isn't the best option? This interface is
essentially a configurable wrapper around the {{Precision}} methods. The
"context" in this case is the configuration that has been applied at
instantiation, i.e. the epsilon value.
bq. there is no need for the AbstractPrecisionComparator when all the methods
can be default methods in the interface
I agree. That is much cleaner.
bq. Or maybe signum(double a) was actually intended , in which case the return
type should also be changed.
I was picturing this as an easy way to compare to zero. You can see it used
[here|https://github.com/apache/commons-geometry/blob/master/commons-geometry-core/src/main/java/org/apache/commons/geometry/core/partitioning/AbstractHyperplane.java#L39].
It's essentially a signum but returning an int for strict equality checking
(eg, {{cmp == 0}}). It could easily be written as
{code:java}
default int sign(double a) {
return compare(a, 0.0);
}
{code}
In regard to the handling of -0.0, that is entirely up to the implementation.
If {{compare(d, -0.0)}} returns 0, then so should this method.
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Attachments: NUMBERS-77.diff
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17327876#comment-17327876
]
Alex Herbert commented on NUMBERS-77:
-
Looking at Matt's PR I would agree that there is no need for the
AbstractPrecisionComparator when all the methods can be default methods in the
interface. I like the implementation from Gilles with a simple interface with
default methods.
All the comparisons in {{sign}} seem redundant:
{code:java}
default int sign(double a) {
final int cmp = compare(a, 0.0);
if (cmp < 0) {
return -1;
} else if (cmp > 0) {
return 1;
}
return 0;
}
{code}
The only reason for all the comparisons is if you expect compare to return a
value that is not in [-1, 0, 1]. If so then the implementation of compare is
wrong. Surely it would be:
{code:java}
default int sign(double a) {
return compare(a, 0.0);
}
{code}
The question is do you wish it to return -1 for -0.0? Using Double.compare(a,
0.0) as the implementation it would do. Using the operators < and > it would
not. Any use of an epsilon should allow it.
So you have to deal with the use of -0.0 and 0.0 and what to expect. This would
be a signum function that handles -0.0 and 0.0 as returning 0.0:
{code:java}
default double signum(double a) {
return (a == 0.0 || compare(a, 0.0) == 0 || Double.isNaN(d)) ? d :
Math.copySign(1.0, d);
}
{code}
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Attachments: NUMBERS-77.diff
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17327190#comment-17327190
]
Gilles Sadowski commented on NUMBERS-77:
I think that the {{sign(double a)}} method should be removed from the API,
being just syntactic sugar for {{compare(a, 0d)}}.
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Attachments: NUMBERS-77.diff
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17327011#comment-17327011
]
Gilles Sadowski commented on NUMBERS-77:
I've just attached a [^NUMBERS-77.diff] with my idea of how to port the
{{DoublePrecisionContext}} functionality. [Maybe the name is still not right.]
Main points are
* the interface (with "default" methods)
* the anonymous instance returned by the factory method
* no {{Serializable}}
* no {{equals}}, {{hashCode}}, {{toString}} methods
The rationale is still to have as lean an API as possible.
Could the functionality be used as is in [Geometry]?
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Attachments: NUMBERS-77.diff
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17326917#comment-17326917
]
Matt Juntunen commented on NUMBERS-77:
--
I had an idea on the name: {{PrecisionComparator}}. I've put up a new version
here: https://github.com/apache/commons-numbers/pull/89.
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17326883#comment-17326883
]
Matt Juntunen commented on NUMBERS-77:
--
bq. PR#88 fails the build.
Yes, I'm not sure what that's about. The failure says that a git commit is
missing:
{code}
Caused by: org.eclipse.jgit.errors.MissingObjectException: Missing commit
99c627e57e5bb14ada31f33809aa62356bfbb7bd
at org.eclipse.jgit.storage.file.WindowCursor.open (WindowCursor.java:126)
at org.eclipse.jgit.revwalk.RevWalk.getCachedBytes (RevWalk.java:856)
{code}
bq. Name is not very self-descriptive (despite its length).
Any suggestions? I've been using this class for so long I can't picture it as
anything else.
bq. Make it an interface (instead of an abstract class)?
Sure.
bq. Make a factory method in Precision class?
Sure.
bq. Is it ever used through Comparator? If so, isn't there a potential
performance issue because of automatic boxing/unboxing.
I believe that only the primitive {{compare}} method is used. It seemed like
since we had the primitive method in place that we should go ahead and
implement the actual {{Comparator}} interface.
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[ https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17326762#comment-17326762 ] Gilles Sadowski commented on NUMBERS-77: PR#88 fails the [build|https://travis-ci.com/github/apache/commons-numbers/builds/223760510]. > Move utilities from "Commons Geometry" > -- > > Key: NUMBERS-77 > URL: https://issues.apache.org/jira/browse/NUMBERS-77 > Project: Commons Numbers > Issue Type: Task >Reporter: Gilles Sadowski >Priority: Major > Fix For: 1.1 > > Time Spent: 0.5h > Remaining Estimate: 0h > > "Commons Geometry" defines utilities that would be a better fit in this > component. > Duplication of general-purpose codes should be avoided, in order to benefit > from consolidated usage (bug reporting, performance enhancements, ...). -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17326761#comment-17326761
]
Gilles Sadowski commented on NUMBERS-77:
bq. {{DoublePrecisionContext}} [is not] geometry-specific.
Sure.
# Name is not very self-descriptive (despite its length).
# Make it an interface (instead of an {{abstract}} class)?
# Make a factory method in {{Precision}} class?
# Is it ever used through {{Comparator}}? If so, isn't there a
potential performance issue because of automatic boxing/unboxing.
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17326563#comment-17326563
]
Matt Juntunen commented on NUMBERS-77:
--
I've created a PR for moving the {{DoublePrecisionContext}} classes from
commons-geometry to commons-numbers-core. It seems like a good fit since there
is nothing geometry-specific in those classes.
https://github.com/apache/commons-numbers/pull/88
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles Sadowski
>Priority: Major
> Fix For: 1.1
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[ https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16720817#comment-16720817 ] Gilles commented on NUMBERS-77: --- bq. my work on Fraction overflows Commenting on the wrong JIRA page? > Move utilities from "Commons Geometry" > -- > > Key: NUMBERS-77 > URL: https://issues.apache.org/jira/browse/NUMBERS-77 > Project: Commons Numbers > Issue Type: Task >Reporter: Gilles >Priority: Major > Fix For: 1.0 > > > "Commons Geometry" defines utilities that would be a better fit in this > component. > Duplication of general-purpose codes should be avoided, in order to benefit > from consolidated usage (bug reporting, performance enhancements, ...). -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[ https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16720515#comment-16720515 ] Eric Barnhill commented on NUMBERS-77: -- Apologies, but I realize belatedly that my work on Fraction overflows has exceeded the scope of the ticket. Propose starting a ticket to deal with these Fraction overflow issues. I think the VALJO issues were wrapped by Stephen Colebourne > Move utilities from "Commons Geometry" > -- > > Key: NUMBERS-77 > URL: https://issues.apache.org/jira/browse/NUMBERS-77 > Project: Commons Numbers > Issue Type: Task >Reporter: Gilles >Priority: Major > Fix For: 1.0 > > > "Commons Geometry" defines utilities that would be a better fit in this > component. > Duplication of general-purpose codes should be avoided, in order to benefit > from consolidated usage (bug reporting, performance enhancements, ...). -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16719159#comment-16719159
]
Gilles commented on NUMBERS-77:
---
I was thinking about utilities that work tuples (e.g. {{SimpleTupleFormat}})
and other array-like structures ({{Interval}} ?).
However, most of those are in {{internal}} package(s); those can be moved them
later if necessary, when the need and API have been evaluated (better not delay
even more the prospective release of "Numbers").
> Move utilities from "Commons Geometry"
> --
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
>Reporter: Gilles
>Priority: Major
> Fix For: 1.0
>
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (NUMBERS-77) Move utilities from "Commons Geometry"
[ https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16719089#comment-16719089 ] Matt Juntunen commented on NUMBERS-77: -- Which utilities should be moved? > Move utilities from "Commons Geometry" > -- > > Key: NUMBERS-77 > URL: https://issues.apache.org/jira/browse/NUMBERS-77 > Project: Commons Numbers > Issue Type: Task >Reporter: Gilles >Priority: Major > Fix For: 1.0 > > > "Commons Geometry" defines utilities that would be a better fit in this > component. > Duplication of general-purpose codes should be avoided, in order to benefit > from consolidated usage (bug reporting, performance enhancements, ...). -- This message was sent by Atlassian JIRA (v7.6.3#76005)
