Hi.

Le lun. 13 janv. 2020 à 04:39, Matt Juntunen
<matt.juntu...@hotmail.com> a écrit :
>
> Gilles,
>
> > How about keeping "Transform" for the interface name and define a method 
> > ... boolean isAffine();
>
> I would prefer to have separate types for each kind of transform.
> This would make the API clear and would avoid numerous checks in the code in 
> order to see if a particular transform instance is supported. The transform 
> types also generally have an "is-a" relationship with each other, which seems 
> like a perfect fit for inheritance. [1]
>
> > I don't get that it is an "accuracy" issue. If some requirement is not met,
> results will be plain wrong
>
> Yes, you are correct. I was not very clear in what I wrote. The results will 
> be completely unusable if the transform does not meet the requirements.
>
> > I wonder why the documented requirement that an "inverse transform
> must exist" does not translate into a method ... getInverse();
>
> Good point. All current implementations are able to provide an inverse so 
> that method should be present on the interface.
>
> In regard to renaming the Transform interface, I had another idea. The main 
> purpose of that interface is to provide a way for the partitioning code in 
> the core module to implement generic transforms of BSP trees (see 
> AbstractBSPTree.transform()). This is what leads to the requirement that the 
> transform preserve parallelism, since otherwise, the represented region may 
> be warped in such a way as to make the tree invalid. However, as far as I can 
> tell, there is not a general mathematical term for this type of transform 
> that applies to Euclidean and non-Euclidean geometries. So, my thought is to 
> move the Transform interface to the "partitioning" package to indicate its 
> relationship to those classes and simply name it something descriptive like 
> "ParallelismPreservingTransform" ("parallelism" since that is the more 
> generic, non-Euclidean form of the concept of "parallel"). The Euclidean 
> module could then provide a true "AffineTransform" interface that extends 
> "ParallelismPreservingTransform". The spherical module transforms would 
> directly inherit from "ParallelismPreservingTransform" and thus avoid any 
> incorrect usage of the term "affine". The class hierarchy would then look 
> like this:
>
> commons-geometry-core
>    ParallelismPreservingTransform
>
> commons-geometry-euclidean
>     AffineTransform extends ParallelismPreservingTransform
>     AffineTransformXD extends AffineTransform
>     AffineTransformMatrixXD implements AffineTransformXD
>     Rotation3D extends AffineTransform3D
>     QuaternionRotation implements Rotation3D
>
> commons-geometry-spherical
>     Transform1S implements ParallelismPreservingTransform<Point1S>
>     Transform2S implements ParallelismPreservingTransform<Point2S>
>
> I think the type names here are much more descriptive and mathematically 
> accurate. WDYT?

I'm not convinced that such a hierarchy would enhance clarity.
If the "Transform" is intimately related to the "core" and there is a single
set of properties that make it "affine" (and work correctly), I'd tend to
keep the name "Transform".  [As long as unit tests ensure that concrete
implementations possess the expected properties, we are safe.]

Regards,
Gilles

> -Matt
>
>
> [1] https://en.wikipedia.org/wiki/Geometric_transformation
>
> ________________________________
> From: Gilles Sadowski <gillese...@gmail.com>
> Sent: Wednesday, January 8, 2020 8:16 AM
> To: Commons Developers List <dev@commons.apache.org>
> Subject: Re: [geometry] Rename Transform to AffineTransform
>
> Hi.
>
> Le mer. 8 janv. 2020 à 04:39, Matt Juntunen
> <matt.juntu...@hotmail.com> a écrit :
> >
> > Gilles,
> >
> > > I thought that the question was how to replace "transform"...
> >
> > I should probably clarify. I want to change the name of the Transform class 
> > to make it clear that it only represents transforms that preserve 
> > parallelism (eg, affine transforms). The most obvious name would be 
> > AffineTransform
>
> How about keeping "Transform" for the interface name and define a method
> ---CUT---
> /**
>  * Move here the doc explaining under what conditions this method can
> return "true".
>  */
> boolean isAffine();
> ---CUT---
> ?
>
> Gilles
>
> > like I suggested but I want to make sure that no one objects to this for 
> > design or mathematical reasons.
> >
> > > Anyways, what would be the issue(s) of a non-affine transform?
> > > IOW why should implementations of "Transfrom" be restricted to affine
> > > transform?
> >
> > Instances of Transform are used to transform hyperplanes and 
> > hyperplane-bounded regions. If the transform is not affine, then the 
> > computed results will not be accurate.
>
> I don't get that it is an "accuracy" issue. If some requirement is not met,
> results will be plain wrong; so it depends on usage: when the transform
> must be affine, the code being passed that instance should be able to
> check whether it can use it for the intended purpose.
>
> I wonder why the documented requirement that an "inverse transform
> must exist" does not translate into a method
> ---CUT---
> Transform<P> getInverse();
> ---CUT---
>
> Regards,
> Gilles
>
> > -Matt
> > ________________________________
> > From: Gilles Sadowski <gillese...@gmail.com>
> > Sent: Tuesday, January 7, 2020 6:41 PM
> > To: Commons Developers List <dev@commons.apache.org>
> > Subject: Re: [geometry] Rename Transform to AffineTransform
> >
> > Le mar. 7 janv. 2020 à 17:55, Matt Juntunen
> > <matt.juntu...@hotmail.com> a écrit :
> > >
> > > Gilles,
> > >
> > > > "AffineMap" (?)
> > >
> > > I think any name that doesn't include the word "transform" somehow would 
> > > probably be confusing.
> >
> > This is supposed to be a synonym.[1]
> > I thought that the question was how to replace "transform"...
> >
> > >
> > > > Was the same "Transform" interface used in both the "euclidean" and the
> > > "spherical" packages of "Commons Math"?
> > >
> > > Indirectly. SphericalPolygonsSet extended AbstractRegion, which included 
> > > an applyTransform(Transform) method.
> >
> > So the "affine" requirement (in the doc) applied there too.
> >
> > Anyways, what would be the issue(s) of a non-affine transform?
> > IOW why should implementations of "Transfrom" be restricted to affine
> > transform?
> >
> > Regards,
> > Gilles
> >
> > [1] https://en.wikipedia.org/wiki/Affine_transformation
> >
> > > -Matt
> > > ________________________________
> > > From: Gilles Sadowski <gillese...@gmail.com>
> > > Sent: Tuesday, January 7, 2020 10:29 AM
> > > To: Commons Developers List <dev@commons.apache.org>
> > > Subject: Re: [geometry] Rename Transform to AffineTransform
> > >
> > > Hello.
> > >
> > > Le mar. 7 janv. 2020 à 16:00, Matt Juntunen
> > > <matt.juntu...@hotmail.com> a écrit :
> > > >
> > > > Hi all,
> > > >
> > > > The documentation for the o.a.c.geometry.core.Transform interface 
> > > > (which comes from the original commons-math version) states that 
> > > > implementations must be affine. Therefore, I think we should rename 
> > > > this interface to AffineTransform to avoid confusion with other types 
> > > > of transforms, such as projective transforms. Potential problems with 
> > > > this are
> > > > - the fact that the JDK already has a class with the same name 
> > > > (java.awt.geom.AffineTransform), and
> > >
> > > "AffineMap" (?)
> > >
> > > > - I'm not sure if the term "affine" can technically be applied to 
> > > > non-Euclidean geometries, such as spherical geometry (there may be 
> > > > nuances there that I'm not aware of).
> > >
> > > Was the same "Transform" interface used in both the "euclidean" and the
> > > "spherical" packages of "Commons Math"?
> > >
> > > Regards,
> > > Gilles
> > >
> > > > Anyone have any insight or opinions on this? I've created GEOMETRY-80 
> > > > to track this issue.
> > > >
> > > > Regards,
> > > > Matt
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to