Skip to content

PrepRawData hold directly a dynamic Eigen Matrix rather than ptr to it

Christos Anastopoulos requested to merge ATLAS-EGamma/athena:PrepRawData_unnneeded_ptr into master

Relates to https://its.cern.ch/jira/projects/ATLASRECTS/issues/ATLASRECTS-6393 where @ssnyder suggests to do something which is similar to what is already done here

We hold

std::unique_ptr<const Amg::MatrixX> m_localCovariance;

And we have ctors with std::unique_ptr or const Amg::MatrixX*.

In this MR try to change to

const Amg::MatrixX m_localCovariance;

in ctors

const Amg::MatrixX* => const Amg::MatrixX&

std::unique_ptr<const Amg::MatrixX> ==> Amg::MatrixX&& retain the move semantics which also could avoid a copy here. e.g should avoid the allocation and a copy.

A lot clients need a set of repetitive changes from pointers to values here so CI will have compilation issues and will have to toggle prb to draft.

Notice the this relies also on this

A default constructor is always available, never performs any dynamic memory allocation, and never initializes the matrix coefficients. You can do:
MatrixXf b;
Here,
    b is a dynamic-size matrix whose size is currently 0-by-0, and whose array of coefficients hasn't yet been allocated at all.

e.g a null prt to Dynamic matrix is now an empty dynamic Eigen matrix .

Mentioning @sroe and @akraszna

PS : another way to see the reason is this (taken from Eigen 3 Abi)

Matrix<T,Dynamic,Dynamic> 
	
struct {
  T *data;                  // with (size_t(data)%EIGEN_MAX_ALIGN_BYTES)==0
  Eigen::Index rows, cols;
 };

e.g a dynamic matrix is alreay a ptr. So what we have now is a unique_ptr, to a ptr, which then points to the payload. So what is tried is to cut down the num of ptrs to get to the payload. It also should get rid of a few new and make_unique and allow for value copy, move semantics.

Edited by Christos Anastopoulos

Merge request reports