You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Luc Maisonobe <Lu...@free.fr> on 2013/12/29 15:26:16 UTC
[math] Clirr problem with some ongoing changes for BSP trees
Hi all,
As part of implementing BSP trees on the sphere (see
<https://issues.apache.org/jira/browse/MATH-1085>). I have split the
org.apache.commons.math3.geometry.Vector<S extends Space> interface by
moving a few of its methods in an upper level interface
org.apache.commons.math3.geometry.Point<S extends Space>.
The rationale behind this change is because in fact we need only a very
small subsets of methods for partitioning (for example we don't need
add, subtract and methods of this type), and because when dealing with
spherical geometry, we are not in a vector space and do not have
addition or subtraction operations.
Of course, the existing code dealing with euclidean geometry implements
both the Point and Vector interfaces, only the new code dealing with
spherical geometry only implements Point, which is sufficient for BSP trees.
So, I have changed the generic interfaces (Hyperplane, SubHyperplane,
Embedding, Region...) and classes (BSPTree, AbstractRegion) in the
geometry.partitioning package to use the Point interface instead of the
Vector interface. Note that none of the interfaces are expected to be
implemented by users, they are only for internal use to allow the single
implementation of BSPTree. In order to preserve compatibility, in all
implementations classes, I have kept the former methods using
the Vector interface alongside with the new methods using the Point
interface, and simply deprecated them.
As an example, the generic hyperplane interface has a getOffset method
which is now defined as follows:
public interface Hyperplane<S extends Space> {
double getOffset(Point<S> point);
// other methods ommitted for clarity
}
Before the change, the method had a Vector<S> parameter.
The specific Euclidean 3D implementation of this interface is the Plane
class, which now has two methods:
public class Plane implements Hyperplane<Euclidean3D> {
@Deprecated
double getOffset(Vector<Euclidean3D> vector) {
return getOffset((Point<Euclidean3D>) vector);
}
double getOffset(Point<Euclidean3D> point) {
...
}
}
Despite the older getOffset(Vector<S>) method is still there, Clirr
keeps complaining about the method having changed its parameter type.
This looks strange to me, and in fact if Clirr keeps complaining, I
would rather completely remove these old methods, as existing calls
would still be resolved correctly since Vector is an extension of Point
(this would mean we keep source compatibility, but not binary
compatibility). I think this is a false positive from Clirr, do you agree?
There are also real errors from Clirr due to the fact the top interface
have changed (using Point instead of Vector). However, since these
interfaces are not meant to be implemented by users (they are technical
interfaces for our internal use, too bad Java does not allow private
inheritance), I don't think this is a problem for a minor release. Do
you agree?
best regards,
Luc
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [math] Clirr problem with some ongoing changes for BSP trees
Posted by Gilles <gi...@harfang.homelinux.org>.
Hello.
On Sun, 29 Dec 2013 15:26:16 +0100, Luc Maisonobe wrote:
> Hi all,
>
> As part of implementing BSP trees on the sphere (see
> <https://issues.apache.org/jira/browse/MATH-1085>). I have split the
> org.apache.commons.math3.geometry.Vector<S extends Space> interface
> by
> moving a few of its methods in an upper level interface
> org.apache.commons.math3.geometry.Point<S extends Space>.
>
> The rationale behind this change is because in fact we need only a
> very
> small subsets of methods for partitioning (for example we don't need
> add, subtract and methods of this type), and because when dealing
> with
> spherical geometry, we are not in a vector space and do not have
> addition or subtraction operations.
>
> Of course, the existing code dealing with euclidean geometry
> implements
> both the Point and Vector interfaces, only the new code dealing with
> spherical geometry only implements Point, which is sufficient for BSP
> trees.
>
> So, I have changed the generic interfaces (Hyperplane, SubHyperplane,
> Embedding, Region...) and classes (BSPTree, AbstractRegion) in the
> geometry.partitioning package to use the Point interface instead of
> the
> Vector interface. Note that none of the interfaces are expected to be
> implemented by users, they are only for internal use to allow the
> single
> implementation of BSPTree. In order to preserve compatibility, in all
> implementations classes, I have kept the former methods using
> the Vector interface alongside with the new methods using the Point
> interface, and simply deprecated them.
>
> As an example, the generic hyperplane interface has a getOffset
> method
> which is now defined as follows:
>
> public interface Hyperplane<S extends Space> {
> double getOffset(Point<S> point);
> // other methods ommitted for clarity
> }
>
> Before the change, the method had a Vector<S> parameter.
>
> The specific Euclidean 3D implementation of this interface is the
> Plane
> class, which now has two methods:
>
> public class Plane implements Hyperplane<Euclidean3D> {
>
> @Deprecated
> double getOffset(Vector<Euclidean3D> vector) {
> return getOffset((Point<Euclidean3D>) vector);
> }
>
> double getOffset(Point<Euclidean3D> point) {
> ...
> }
>
> }
>
> Despite the older getOffset(Vector<S>) method is still there, Clirr
> keeps complaining about the method having changed its parameter type.
> This looks strange to me, and in fact if Clirr keeps complaining, I
> would rather completely remove these old methods, as existing calls
> would still be resolved correctly since Vector is an extension of
> Point
> (this would mean we keep source compatibility, but not binary
> compatibility). I think this is a false positive from Clirr, do you
> agree?
Yes.
This had already happened (with method "cumulativeProbability" in the
"RealDistribution", IIRC).
>
> There are also real errors from Clirr due to the fact the top
> interface
> have changed (using Point instead of Vector). However, since these
> interfaces are not meant to be implemented by users (they are
> technical
> interfaces for our internal use, too bad Java does not allow private
> inheritance), I don't think this is a problem for a minor release. Do
> you agree?
You are the one who knows best what users should not have done with
this
code. :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org