Hello.
2020-01-18 15:40 UTC+01:00, Matt Juntunen <matt.juntu...@hotmail.com>:
> Gilles,
>
>> 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".
>
> So, if I'm understanding you correctly, you're saying that since the
> partitioning code in the library only works with these types of
> parallelism-preserving transforms, it can be safely assumed that
> o.a.c.geometry.core.Transform represents such a transform. Is this correct?
Indeed.
> One thing that's causing some issues with the implementation here is that
> the Euclidean TransformXD interfaces have static "from(UnaryOperator<X>)"
> methods that allow users to wrap their own, arbitrary vector operations as
> Transform instances. We don't (and really can't) do any validation on these
> user-defined functions to ensure that they meet the library requirements. It
> is therefore easy for users to pass in invalid operators. To avoid this, I'm
> thinking of removing the TransformXD interfaces completely and moving the
> "from(UnaryOperator<X>)" methods into the AffineTransformMatrixXD classes.
+1
It is generally good to prevent the creation of invalid objects.
> There, we can simply sample the user-defined function
I'm not sure I understand.
> as needed and produce
> matrices that are guaranteed to be affine.
Throwing an exception if the transform does not abide by
the requirements?
> Following the above, the class hierarchy would then be as below, which is
> basically what it was before I added the TransformXD interfaces.
>
> commons-geometry-core
> Transform
>
> commons-geometry-euclidean
> EuclideanTransform extends Transform
> AffineTransformMatrixXD implements EuclideanTransform
> Rotation3D extends EuclideanTransform
> QuaternionRotation implements Rotation3D
>
> commons-geometry-spherical
> Transform1S implements Transform
> Transform2S implements Transform
>
> WDYT?
+1
Best,
Gilles
>
> -Matt
>
>
> ________________________________
> From: Gilles Sadowski <gillese...@gmail.com>
> Sent: Monday, January 13, 2020 8:03 PM
> To: Commons Developers List <dev@commons.apache.org>
> Subject: Re: [geometry] Rename Transform to AffineTransform
>
> 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
