eic / EICrecon

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

Cluster axis direction #1391

Closed sebouh137 closed 3 months ago

sebouh137 commented 4 months ago

Added code to produce the direction of clusters in CalorimeterClusterRecoCoG

Briefly, what does this PR introduce?

adds the cluster direction (i.e., the eigenvector corresponding to the largest eigenvector of the matrix corresponding to the inertia matrix of the cluster)

What kind of change does this PR introduce?

Please check if this PR fulfills the following:

Does this PR introduce breaking changes? What changes might users need to make to their code?

no

Does this PR change default behavior?

Yes. While calculating the shape parameters, the hits are assigned weights, in a similar manner to how they are assigned weights when determining the cluster position. Previously, there were some inconsistencies in how these weights were determined when using certain options in the cluster fitting, which were fixed in order to make this feature (the cluster axis direction) work properly.

github-actions[bot] commented 4 months ago

Capybara summary for PR 1391

sebouh137 commented 3 months ago

@wdconinc I am not sure why the tests is failing. It does not appear to be anything on my end. Could you check if this is something on my end or something outside the scope of this pull request? Best regards, Sebouh

sebouh137 commented 3 months ago

@wdconinc I am not sure why the tests is failing. It does not appear to be anything on my end. Could you check if this is something on my end or something outside the scope of this pull request? Best regards, Sebouh

I got this to pass the checks that were failing, with @nathanwbrei 's help. Now it is passing all checks.

veprbl commented 3 months ago

@sebouh137 I have some second thoughts about this. First of all, could we have used intrinsicEta inrinsicPhi fields? Second, could you, please, update the documentation https://github.com/eic/EDM4eic/blob/3b4975588356d81321b396490fe67fe31e438ac1/edm4eic.yaml#L292 to reflect new status-quo.