coresoftware icon indicating copy to clipboard operation
coresoftware copied to clipboard

MVTX modified geometry (coresoftware)

Open hrjheng opened this issue 1 year ago • 13 comments

Types of changes

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [x] Breaking change (fix or feature that would cause existing functionality to not work for users)
  • [x] Requiring change in macros repository (Please provide links to the macros pull request in the last section)
  • [x] I am a member of GitHub organization of sPHENIX Collaboration, EIC, or ECCE (contact Chris Pinkenburg to join)

What kind of change does this PR introduce? (Bug fix, feature, ...)

  • Modify MVTX GEANT geometry based on the alignment parameters (i.e "localAlignmentParamFile.txt")
  • A new PHG4MvtxMisalignment class that calculates the global displacements of MVTX
  • MakeActsGeometry: fix matching between the ActsSurface and TrkrDef::hitsetkey to account for the global shift of MVTX

Software and simulation meeting on 20241001: https://indico.bnl.gov/event/24940/#14-mvtx-geometry-update

TODOs (if applicable)

Links to other PRs in macros and calibration repositories (if applicable)

  • macros: https://github.com/sPHENIX-Collaboration/macros/pull/956
  • calibrations: https://github.com/sPHENIX-Collaboration/calibrations/pull/158

hrjheng avatar Oct 01 '24 15:10 hrjheng

Build & test report

Report for commit 7f3d09756b635f3f43fab4932a0b8e2e439e4b0c: Jenkins passed


Automatically generated by sPHENIX Jenkins continuous integration sPHENIX             jenkins.io

sphenix-jenkins-ci[bot] avatar Oct 01 '24 19:10 sphenix-jenkins-ci[bot]

Build & test report

Report for commit 2a6f1a2d0bca9c26a4b0afa3cda38413ce7db9b9: Jenkins passed


Automatically generated by sPHENIX Jenkins continuous integration sPHENIX             jenkins.io

sphenix-jenkins-ci[bot] avatar Oct 02 '24 01:10 sphenix-jenkins-ci[bot]

Build & test report

Report for commit bbe70dd664145ebf2f26fb3b4d9c5e7987a674b9: Jenkins passed


Automatically generated by sPHENIX Jenkins continuous integration sPHENIX             jenkins.io

sphenix-jenkins-ci[bot] avatar Oct 02 '24 02:10 sphenix-jenkins-ci[bot]

Looking at these clang-tidy issues locally, they are both warnings and not actual errors so I'm not sure why Jenkins flags these as problematic. Regardless, one of them according to stack overflow can be ignored if you really intend to call the function recursively

osbornjd avatar Oct 02 '24 12:10 osbornjd

The recursive is just a warning (it is a possible infinite loop if you call it the wrong way and clang-tidy assumes the worst). If you add this // NOLINTNEXTLINE(misc-no-recursion) right before void PHG4MvtxDetector::SetDisplayProperty(G4LogicalVolume *lv) it suppresses it The other is the use of int active. clang-tidy makes a good point that for bit operations you really should not use ints where the sign bit can mess with you. It looks like active can (should) be an unsigned int. As soon as you change that this error should be gone (depending on what m_IsLayerActive returns, if it's int's there are more problems because bit fiddling using a signed int is error prone as well)

pinkenburg avatar Oct 02 '24 13:10 pinkenburg

m_IsLayerActive returns int as std::array<int, n_Layers> m_IsLayerActive{}; and the std::array is set by m_IsLayerActive[ilayer] = params->get_int_param("active");. I'll just add // NOLINTNEXTLINE(misc-no-recursion) for now

And to add that m_IsLayerActive is not a new object I introduced. It was already in the PHG4MvtxDetector

hrjheng avatar Oct 02 '24 17:10 hrjheng

Build & test report

Report for commit 0d278efc37326afbfb2484a93404745fd9569b02: Jenkins passed


Automatically generated by sPHENIX Jenkins continuous integration sPHENIX             jenkins.io

sphenix-jenkins-ci[bot] avatar Oct 02 '24 22:10 sphenix-jenkins-ci[bot]

Build & test report

Report for commit a6a1b02ccc5ab664d6436912621c24052872ebf8: Jenkins passed


Automatically generated by sPHENIX Jenkins continuous integration sPHENIX             jenkins.io

sphenix-jenkins-ci[bot] avatar Oct 04 '24 09:10 sphenix-jenkins-ci[bot]

Build & test report

Report for commit c4fbad0a8f67cafc2dc7c4f7e95826693dcfcc3c: Jenkins passed


Automatically generated by sPHENIX Jenkins continuous integration sPHENIX             jenkins.io

sphenix-jenkins-ci[bot] avatar Oct 04 '24 19:10 sphenix-jenkins-ci[bot]

This is the latest build with the macros changes too https://web.sdcc.bnl.gov/jenkins-sphenix/job/sPHENIX/job/test-tracking-low-occupancy-qa/6165/ which show large changes to the QA. We need to understand why this happens as I thought the flags were all set to use the ideal geometry still? So I thought we should get identical performance

osbornjd avatar Oct 04 '24 19:10 osbornjd

This is the latest build with the macros changes too https://web.sdcc.bnl.gov/jenkins-sphenix/job/sPHENIX/job/test-tracking-low-occupancy-qa/6165/ which show large changes to the QA. We need to understand why this happens as I thought the flags were all set to use the ideal geometry still? So I thought we should get identical performance

Are you referring to the tpc, tracking, and vertex QAs that show large discrepancies and low KS-test scores?

hrjheng avatar Oct 04 '24 19:10 hrjheng

Correct, my understanding was that the flags were turned off which effectively leaves the MVTX in its current unmodified state, meaning we would expect no change in the QA. Is my understanding wrong?

osbornjd avatar Oct 04 '24 19:10 osbornjd

It's my understanding too that MVTX should be in the ideal and default position. Curiously INTT and MVTX QA do not change (not sure about TPOT). The only place that might affect tracking is MakeActsGeometry, but I only modified the part relevant to MVTX, void MakeActsGeometry::makeMvtxMapPairs(TrackingVolumePtr &mvtxVolume). I see a few changes to the macro repository that I have not pulled into my branch. I am not sure if it will fix the issue but just want to make sure everything is the same

hrjheng avatar Oct 04 '24 20:10 hrjheng

Build & test report

Report for commit 623b4d909cc4bf4753b52db173a82b8e9e2e4ef2: Jenkins on fire


Automatically generated by sPHENIX Jenkins continuous integration sPHENIX             jenkins.io

sphenix-jenkins-ci[bot] avatar Oct 09 '24 19:10 sphenix-jenkins-ci[bot]

Build & test report

Report for commit 14930d0bec5f53d60059f7bfc30bdde81f80bf70: Jenkins on fire


Automatically generated by sPHENIX Jenkins continuous integration sPHENIX             jenkins.io

sphenix-jenkins-ci[bot] avatar Oct 09 '24 19:10 sphenix-jenkins-ci[bot]

Build & test report

Report for commit f8b4327f62b3c73da1d4164262c1c13e9b524d44: Jenkins passed


Automatically generated by sPHENIX Jenkins continuous integration sPHENIX             jenkins.io

sphenix-jenkins-ci[bot] avatar Oct 09 '24 20:10 sphenix-jenkins-ci[bot]

Successful build here https://web.sdcc.bnl.gov/jenkins-sphenix/job/sPHENIX/job/Build-Master-gcc12/3891/

osbornjd avatar Oct 16 '24 17:10 osbornjd