Optimised point

[VyasGuru]

Optimize point class arithmetic

Summary

This PR introduces significant performance optimizations to the core point class.

Changes

Performance Optimizations

• File: include/cartesian_geom/point.h

• 1-> (squared_length):

  • Replaced return length() * length() with return coeffs.squaredNorm().
  • Benefit: Eliminates an expensive square root instruction followed by a multiplication. This improves performance for distance-based checks and increases precision by avoiding floating-point errors.

• 2->(Overloading):

  • Added template overloads for operator+= and operator-= that accept Eigen::MatrixBase<Derived>.
  • Benefit: Enables lazy evaluation, improving performance by reducing memory load.

• From running the benchmarks, I have found there to be a speedup of about 10% ( 9~12), which i think is pretty significant for some of the much heavier computations that occur.

Adresses #434

Currently this runs into an issue with two tests:

1: volume_cg_hpolytope_simplex
2: test_max_ball_sparse

The reason for the issue is that the previous implementation required a temp variable to be created when doing a step like a+= bc. Here, bc used to be temporarily stored and then added to a. This causes two rounding operations: one during the b*c process and one during the addition. In a sense, this was less accurate.

The change I have made now only does one rounding operation, but due to this, the values don't match what the tests are expecting.

@vissarion, how would you suggest moving forward with this?

[vissarion]

@VyasGuru thanks for this PR. Why did you close #435 that look identical to this one?

[VyasGuru]

@VyasGuru thanks for this PR. Why did you close #435 that look identical to this one?

That was an error on my side. Apologies for the confusion.
Could you please guide on how to move about with this test discrepancy?

[vissarion]

is this check needed?

[VyasGuru]

I had initially put that in, keeping in mind this bit of code from hpolytope.h

 void compute_reflection(Point& v, Point const&, int const& facet) const
    {
        v += -2 * v.dot(A.row(facet)) * A.row(facet);
    }

to my knowledge, a row vector was being added to a column vector, which is why the tranpose was needed. But I tried it out, removing the check and it seems to work? perhaps it has something to do with how Eigen handles it, but yes, the check seems to not be necessary.

[vissarion]

It is not clear where are the implementations of template overloads for operators.