eic / EICrecon

EIC Reconstruction - JANA based
https://eic.github.io/EICrecon
GNU Lesser General Public License v3.0
5 stars 24 forks source link

CalorimeterClusterRecoCoG: set intrinsic{Phi,Theta} #1511

Open veprbl opened 2 weeks ago

veprbl commented 2 weeks ago

This partially reverts effect of #1391. The clusterShape has become a way to extend EDM without going through extending it. We should sort those variable out into proper members, this is a first step.

github-actions[bot] commented 2 weeks ago

Capybara summary for PR 1511

wdconinc commented 1 week ago

(Outside of reviewable context)

This is a symmetric real (positive semi-definite), i.e. self-adjoint problem, so the Eigen::EigenSolver<Eigen::Matrix2f> and Eigen::EigenSolver<Eigen::Matrix3f> on 2D and 3D covariance matrices can be replaced by Eigen::SelfAdjointEigenSolver<Eigen::Matrix2f> and Eigen::SelfAdjointEigenSolver<Eigen::Matrix3f>. This then allows to change Eigen::Vector2cf eigenValues_2D and Eigen::Vector3cf eigenValues_3D to purely real Eigen::Vector2f eigenValues_2D and Eigen::Vector3f eigenValues_3D, and there is no need to explicit real calls or carrying zero imaginary parts.

Since the quadratic forms that add to the covariance matrix are symmetric, we only need to compute/store the lower triangular part, but it doesn't seem like Eigen3 provides a self-adjoint matrix class itself (only a view), so I don't think this can be simplified further, computationally.

sonarcloud[bot] commented 1 week ago

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
1.6% Duplication on New Code

See analysis details on SonarCloud