Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Factor out tracking, update all CIs to full sequence #1551

Closed
wants to merge 16 commits into from

Conversation

tvami
Copy link
Member

@tvami tvami commented Jan 30, 2025

I am updating ldmx-sw, here are the details.

What are the issues that this addresses?

This resolves #1549

Check List

  • I successfully compiled ldmx-sw with my developments
  • I ran my developments and the following shows that they are successful.

I tested with the ecal PN sample.

I did two things in this PR

  1. update the CIs to use the latest tracking, for this I made a new file so I dont have to copy a lot of things, I also updated reco.py @bloodyyugo I took the numbers from Add ambiguity resolution to tracking #1526 (and rounded them).

  2. Ecal Veto now relies on the tracking for the photon projection, when it can (i.e. if a recoil track exists). With this we also introduced another ep angle as measured at the target, or as it used to be measured at the ECAL face. Much of this work was done by @JYoo001

@tvami tvami force-pushed the iss1549-latest-track-in-ecal branch from dbb0de1 to 3c474fd Compare January 30, 2025 20:50
@tvami tvami force-pushed the iss1549-latest-track-in-ecal branch from 3c474fd to 61bf3e7 Compare January 30, 2025 20:51
@tvami tvami marked this pull request as draft January 30, 2025 21:02
@tvami tvami marked this pull request as ready for review January 30, 2025 21:02
@tvami tvami marked this pull request as draft January 31, 2025 01:52
@tvami tvami marked this pull request as ready for review January 31, 2025 01:52
@tvami tvami marked this pull request as draft January 31, 2025 15:03
@tvami tvami marked this pull request as ready for review January 31, 2025 15:03
@tvami
Copy link
Member Author

tvami commented Jan 31, 2025

Maybe I should have done this in two steps...

But let's concentrate on the tracking changes first:
[the followings are for the kaon sample, for the recoil DQM variables]

  1. I dont understand the dip in the middle of this distribution:
Screenshot 2025-02-01 at 13 09 15
  1. Recoil layer hits are gone??
Screenshot 2025-01-31 at 08 37 47
  1. The match nHits change I could expect from the ambiguity resolution
Screenshot 2025-02-01 at 13 09 33
  1. Number of tracks has a big jump at the zero bin
Screenshot 2025-01-31 at 08 39 59
  1. NDF got higher
Screenshot 2025-01-31 at 08 41 07
  1. Sigma p vs p got better
Screenshot 2025-01-31 at 08 42 20
  1. But the pull is worse
Screenshot 2025-01-31 at 08 43 48
  1. I must have missed a setting for the target projection, bc the histograms are empty, I'll check this asap
Screenshot 2025-01-31 at 08 45 37
  1. More duplicates
Screenshot 2025-01-31 at 08 46 40

[Now moving to the tagger]

  1. Wow the tagger d0 is shifted a lot, is that real or did I mistype some number??
Screenshot 2025-01-31 at 08 49 25
  1. All the duplicates plots are empty for the tagger

  2. Same for layer hits and matched layer hits

  3. Num of tracks in the tagger looks good tho!

Screenshot 2025-01-31 at 08 51 35
  1. NDF is all at 10 now
Screenshot 2025-01-31 at 08 51 56
  1. Together with the number of hits, that's all at 14 now

  2. The pT might be just because we get the track in an ange?

Screenshot 2025-01-31 at 08 53 41
  1. The "pZ" looks good
Screenshot 2025-01-31 at 08 54 23
  1. Pull d0 is gone, as is the phi d0
Screenshot 2025-01-31 at 08 55 07
  1. The z0 got much wider
Screenshot 2025-01-31 at 08 55 49
  1. Note to self: I forgot to add tracking to the signal CI

@EBerzin @bloodyyugo are these changes according to your expectations?

@tvami
Copy link
Member Author

tvami commented Feb 1, 2025

The validation workflow is running here: https://github.com/LDMX-Software/ldmx-sw/actions/runs/13081772275.

OK so in this I kept the CKF the usual DQM, and introduced new ones for the GSF. Things pass the K-S test... This is a bit surprising to be since I changed to the digis + changed the variables. But if it's true that means everything in my prev comment comes from the combination of the ambiguity resolution and the GSF

@tvami tvami marked this pull request as draft February 1, 2025 04:38
@tvami tvami marked this pull request as ready for review February 1, 2025 04:38
@tvami
Copy link
Member Author

tvami commented Feb 1, 2025

Things pass the K-S test... This is a bit surprising to be since I changed to the digis + changed the variables

Ignore all the workflows that got triggered by my command line testing, it doesnt check out the correct branch. Now I triggered test with the usual "go to draft, ready for review" way

@tvami
Copy link
Member Author

tvami commented Feb 1, 2025

Well this has not much to do with this PR's developments, but I guess it got revealed it now:

[ ecalVeto ] 122  info:  Recoil tracking for projections; with 1 tracks, fiducial in tracker = true
  [ ecalVeto ] 122  info: Finding linreg tracks from a total of 6 hits using a radius of 35 mm
  [ ecalVeto ] 122  info:  MIP tracking completed; found 0 straight tracks and 0 lin-reg tracks
  [ ecalVeto ] 122  info:   BDT was ran, score is 0.999697
  [ hcalVeto ] 122  info: There are 0 / 0 HCal hits read out. 0 are not associated with the recoil ele. Total PE of 0
  [ hcalVeto ] 122  info: HCAL veto passed? true
  [ ElectronCounter ] 122  info: Found 1 electrons (tracks) using input collection TriggerPadTracksY_
  [ trigger ] 122  info: Got trigger energy cut 3000 for 1 electrons counted in the event.
  [ trigger ] 122  info: Got trigger energy sum 807.849; and decision is pass = true
  05:29:18    PROP           ERROR     Step failed with MagneticFieldError:1: Interpolation out of bounds was requested
  
   *** Break *** segmentation violation

@LDMX-Software LDMX-Software deleted a comment from github-actions bot Feb 1, 2025
@tvami tvami marked this pull request as draft February 2, 2025 15:53
@tvami tvami marked this pull request as ready for review February 2, 2025 15:53
@tvami
Copy link
Member Author

tvami commented Feb 2, 2025

I will delay adding the tracking to the signal, and made an issue about it: #1562

@tvami
Copy link
Member Author

tvami commented Feb 4, 2025

/run-validation

Copy link
Contributor

github-actions bot commented Feb 4, 2025

The validation workflow is running here: https://github.com/LDMX-Software/ldmx-sw/actions/runs/13140200480.

@tvami
Copy link
Member Author

tvami commented Feb 6, 2025

/run-validation

Copy link
Contributor

github-actions bot commented Feb 6, 2025

The validation workflow is running here: https://github.com/LDMX-Software/ldmx-sw/actions/runs/13188891225.

@tvami tvami marked this pull request as draft February 10, 2025 04:31
@tvami tvami marked this pull request as ready for review February 10, 2025 04:32
@tvami tvami marked this pull request as draft February 10, 2025 04:33
@tvami tvami marked this pull request as ready for review February 10, 2025 04:33
@tvami
Copy link
Member Author

tvami commented Feb 10, 2025

Validation with the version where the min num hit is back to 6:
https://github.com/LDMX-Software/ldmx-sw/actions/runs/13233604311/job/36934626779?pr=1551

@tvami tvami changed the title Factor out tracking, update all CIs to full sequence, use it in ECAL veto Factor out tracking, update all CIs to full sequence Feb 10, 2025
@tvami
Copy link
Member Author

tvami commented Feb 12, 2025

I will close this PR and have the content done in pieces

@tvami tvami closed this Feb 12, 2025
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.

Update to latest tracking in CIs and use it in ECAL projections
1 participant