Skip to content

Partial fix to HGCal hexagons position, and SimCluster proxy#131

Open
tcuisset wants to merge 3 commits intoalja:mainfrom
tcuisset:pr01
Open

Partial fix to HGCal hexagons position, and SimCluster proxy#131
tcuisset wants to merge 3 commits intoalja:mainfrom
tcuisset:pr01

Conversation

@tcuisset
Copy link

Hi,
I've made a temporary fix for the HGCal hexagon positions, fixing partially issue #128
Now the center of the hexagons are in the correct position (before it seems they were rotated around the origin). It is still missing the rotation around the center of the hexagon (though that is less critical).

I've also added a proxy for SimCluster (very similar to CaloParticle), and a small interface convenience for trackster layer display mode.

image

(the hexagons are drawn in the correct location now, but they are missing a rotation around the hexagon center)
Fixes partially alja#128
@alja
Copy link
Owner

alja commented Dec 1, 2025

@tcuisset

Thank you for the research on mispositioned boxsets!

I looked at the REveBoxSet::AddHex interface and saw that it is not compatible with the old TEveBoxSet::AddHex interface. The REveBoxSet::AddHex expects the rotation angle in radians instead of degrees. I made a simple root macro that tests the orientation of the hexagon here: http://xrd-cache-1.t2.ucsd.edu/alja/mail/boxset_hex.C.

Can you check if you check the digit position is correct if the rotation angle is set angle to TMath::Pi() * 0.5? Here is an example to test a single digit:
root.exe boxset_hex.C

Let me know if this solves the problem.
If not, please send me your data file and instructions for reproducing the problem.

@tcuisset
Copy link
Author

tcuisset commented Dec 3, 2025

I tried changing the angle parameter to pi/2, and it does not solve the problem. The radians-degrees issue might be a separate issue. See the screen capture below where I changed the angle of different collections (the most correct one is angle=0)
RotationHex

The issue is that the rotation angle seems to induce a rotation around the z-axis instead of a rotation around the hexagon center. I've put in a drawing what seems to be the issue:

Hexagon rotation (1)

Here is the link to the input file and geometry file in case you want to test.

@alja
Copy link
Owner

alja commented Dec 4, 2025

@tcuisset
Thank Theo!
I will run the test using the same input and geometry files.
Which CMSSW release are you using?

@tcuisset
Copy link
Author

tcuisset commented Dec 4, 2025

I'm using CMSSW_15_1_0_pre4_ROOT636 for fireworks & geometry
(and the samples were made with CMSSW_16_0_0_pre2 though they seem compatible)

@alja
Copy link
Owner

alja commented Dec 6, 2025

@tcuisset

The bug was in REveBoxSet, where rotation should be in local instead of the parent frame:
alja/root@3b6275c

Fixing the bug in root may also be an opportunity to switch to degrees instead of radians, as in TEve. What units in rotation should REveBoxset::AddHaxagon have, in your opinion?

I will update the FireworksWeb service to include the ROOT correction next week.
The bugfix will also go to the ROOT main branch.
I believe new CMSSW builds with the root master branch are available every second week.

How do you set up a developer environment for FireworksWeb?

@tcuisset
Copy link
Author

tcuisset commented Dec 8, 2025

Hi @alja
Thanks for finding the fix !
For degrees vs radians I don’t have a particular opinion (as long as it’s documented). I guess you can take whatever convention is most used in ROOT/REve for rotations.

To setup a developper environment I just install a CMSSW with a ROOT636 build. I guess to test the fix I can switch to the IB with ROOT master. Then I just clone the FireworksWeb repo, scram build and run (then SSH port forward).

@alja
Copy link
Owner

alja commented Jan 27, 2026

@tcuisset The related PR root-project/root#20670 was merged today. It takes 2 week cycle to integrate the ROOT change in CMSSW root master build. I will update fireworks web service soon after.

@tcuisset
Copy link
Author

Hi @alja ,
I have tested the ROOT fix (on IB CMSSW_16_1_ROOT6_X_2026-03-09-2300) and it seems to work:
image
(I had to fix some compilation errors related to nlohmann json to get it to work)
Though I think the fireworks.web.cern.ch was not yet updated.

There still seems to be a rotation issue, but only in the "cell-local" frame, so it is not really a big issue. See below: the hexagons all have the same orientation, while in HGCAL the cells are rotated to make an optimal paving (without gaps and overlaps).
Screenshot from 2026-03-10 13-54-03

@alja
Copy link
Owner

alja commented Mar 10, 2026

@tcuisset
I was thinking of sending you an update, but it looks like you have already found a way to find CMSSW with the root master build.

I have also made a CMSSW_16_1_ROOT6_X FWLite and FireworksWeb build using one of the recent nightly builds on my el9 desktop. I did not have a compilation problem with JSON, but I had to make a minor change in association collections for the build to pass: 288e779

My build is ready for the service update, but I'm not sure whether I should wait for other development (camera settings and TGEo geometry browser) changes before the update.

Would it be helpful for you to have the service update ready soon?

@alja
Copy link
Owner

alja commented Mar 10, 2026

There still seems to be a rotation issue, but only in the "cell-local" frame, so it is not really a big issue. See below: the hexagons all have the same orientation, while in HGCAL the cells are rotated to make an optimal paving (without gaps and overlaps).

This seem to be translation problem. Below is an image of hexagon tight fit:
Screenshot From 2026-03-10 11-00-29

Is it possible placement is not correct in reco geometry?

@tcuisset
Copy link
Author

Would it be helpful for you to have the service update ready soon?

Personally I do not really need it as I use my local installation, but I think people in HGCAL reconstruction might appreciate having the event display working for hexagons.

Is it possible placement is not correct in reco geometry?

It is a possibility. We could try running on the latest geometry (D121) to see.

@alja
Copy link
Owner

alja commented Mar 19, 2026

@tcuisset
I have made today an update of fireworks.cern.ch service with the latest changes in root.
Please report any issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants