Kernel_23: Add NonZeroDimension_3

[afabri]

Summary of Changes

Add a function object to the concept Kernel that enables to find any dimension of a 3D vector that is different from zero.

• Document the model

Release Management

• Affected package(s): Kernel_23
• Feature/Small Feature (if any): link
• Link to compiled documentation: link
• License and copyright ownership: not changed

[afabri]

/build:v1

[github-actions]

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/6102/v1/Manual/index.html

[afabri]

@sloriot why is the added concept not on this page?

[afabri]

/build:v2

[github-actions]

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/6102/v2/Manual/index.html

[afabri]

/build:v3

[github-actions]

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/6102/v3/Manual/index.html

[MaelRL]

Shouldn't this be called Non_zero_coordinate_3 rather? Using "dimension" is abusing nomenclature, and it's confusing given that there exists Vector_3::dimension() = 3

[lrineau]

Shouldn't this be called Non_zero_coordinate_3 rather? Using "dimension" is abusing nomenclature, and it's confusing given that there exists Vector_3::dimension() = 3

I agree. Here the input of the functor is a Vector_3, and Non_zero_coordinate_3 seems a better naming.

I would understand "non zero dimension" if the input was something like a Bbox_3.

[afabri]

Note that it does not return a coordinate but the index of the dimension. Concerning the name I am wondering it it should be prefixed with Any_.

[afabri]

What about Any_non_zero_coordinate_index_3?

[sloriot]

I don't like the any prefix.

[afabri]

So, do we converge to Non_zero_coordinate_index_3?

[sloriot]

fine with me

[lrineau]

So, do we converge to Non_zero_coordinate_index_3?

It seems a good name.

[afabri]

/force-build:v3

[afabri]

/force-build:v3

[afabri]

/force-build:v3

[github-actions]

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/6102/v3/Manual/index.html

[github-actions]

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/6102/v3/Manual/index.html

[lrineau]

Interdependently from its documentation, about the implementation of Non_zero_coordinate_index_3, should not it return the index of the axis for which the vector has the biggest absolute value? That could increase the performance of the code using that index.

Let's consider we have a set of 3D points that are almost vertical, in the XZ plane. The fitting plane of that point set would have the orthogonal vector v={small_x, big_y, small_z}, where small_x and small_z have small, but non-zero, absolute values. Then, with the current implementation, Non_zero_coordinate_index_3()(v) might return 0, because the value is small, but big enough that certainly_not(is_zero(v.x())) is true. In that case, the bounded_side_2 would be called with a projection on the YZ plane, where the point set is almost degenerated, instead of the plane XZ. If the implementation of Non_zero_coordinate_index_3 tries to return the index of the largest coordinate, it would be 1, instead of 0.

Also, as I said in a private conversion with Andreas, the code should use hx()/hy()/hz(), instead of x()/y()/z(), so that it is as efficient in the homogeneous case as in the cartesian case.

[afabri]

The main gain comes from the fact that we avoid filter failures. I won't explore searching for the biggest for now. I will do the hx() etc.

[lrineau]

This small feature is not yet approved, and yet the release 5.4-beta1 was due two weeks ago. I think this small feature is really late, and should not go into CGAL-5.4.

In particular, this PR conflicts with the preparation of CHANGES.md: #6122.

[sloriot]

Successfully tested in CGAL-5.4-Ic-95