Skip to content
New issue

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

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

Already on GitHub? # to your account

move ParticleDataTable uses to new ParticleDataList #680

Merged
merged 1 commit into from
Jan 7, 2022

Conversation

rlcee
Copy link
Collaborator

@rlcee rlcee commented Dec 22, 2021

Switch to new, simpler ParticleDataList. Validation tests

  • 1000 ceSimReco
  • 500 potSim
  • 300 reco
    show exact matches. I know this is a precise test because a few small diffs in precision with the old system did show differences in the ceSimReco and reco validation.

I also checked uses in TrackerAlignment, REve and TrkAna, and there will be one follow-on commit in TrkAna.

@FNALbuild
Copy link
Collaborator

Hi @rlcee,
You have proposed changes to files in these packages:

  • CRVAnalysis
  • CommonMC
  • TrackerMC
  • GlobalConstantsService
  • Mu2eUtilities
  • Mu2eKinKal
  • Filters
  • EventGenerator
  • CRVResponse
  • Print
  • Analyses
  • CosmicReco
  • CaloFilters
  • Sources
  • ExtinctionMonitorFNAL
  • Compression
  • Mu2eBTrk
  • Trigger
  • TrkReco
  • DAQ
  • DataProducts
  • TrkFilters
  • TrackerConditions
  • CaloDiag
  • ConditionsService
  • EventDisplay
  • Mu2eG4
  • TrkDiag
  • TEveEventDisplay
  • CaloMC

which require these tests: build.

@Mu2e/write, @Mu2e/fnalbuild-users have access to CI actions on main.

The following users requested to be notified about changes to these packages:
@resnegfk

⌛ The following tests have been triggered for 3ebb0ef: build (Build queue is empty)

About FNALbuild. Code review on Mu2e/Offline.

@FNALbuild
Copy link
Collaborator

☀️ The tests passed at 3ebb0ef.

Test Result Details
merge Merged 3ebb0ef at 7cc7f0e
build (prof) Log file. Build time: 14 min 00 sec
ceSimReco Log file.
g4test_03MT Log file.
transportOnly Log file.
POT Log file.
g4study Log file.
cosmicSimReco Log file.
cosmicOffSpill Log file.
ceSteps Log file.
ceDigi Log file.
muDauSteps Log file.
ceMix Log file.
rootOverlaps Log file.
g4surfaceCheck Log file.
FIXME, TODO 〰️ TODO (14) FIXME (15) in 110 files
clang-tidy 〰️ 0 errors 0 warnings

N.B. These results were obtained from a build of this Pull Request at 3ebb0ef after being merged into the base branch at 7cc7f0e.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

@kutschke
Copy link
Contributor

I also checked uses in TrackerAlignment, REve and TrkAna, and there will be one follow-on commit in TrkAna.

WHen you say that you checked TrackerAlignment and REve, do you mean that they do not currently use the old system and therefore need no maintenance? Is that correct?

I see the PR in TrkAna.

Have you looked at TrackerMCTune?

Outside of this PR the appropriate people will need to look at Tutorial and Stntuple.

@rlcee
Copy link
Collaborator Author

rlcee commented Dec 22, 2021

I also checked uses in TrackerAlignment, REve and TrkAna, and there will be one follow-on commit in TrkAna.

WHen you say that you checked TrackerAlignment and REve, do you mean that they do not currently use the old system and therefore need no maintenance? Is that correct?

Yes

I see the PR in TrkAna.

Have you looked at TrackerMCTune?

TrackerMCTune doesn't use ParticleDataTable

Outside of this PR the appropriate people will need to look at Tutorial and Stntuple.

Tutorial doesn't use ParticleDataTable

@kutschke
Copy link
Contributor

Thanks Ray. I am surprised that Tutorial does not use ParticleDataTable but it's certainly simpler that it does not.

@rlcee
Copy link
Collaborator Author

rlcee commented Jan 6, 2022

#672 references the ParticleDataTable include, which should be removed since it is not used. This can be done independently from this PR.

If I run the geant particle print routine, which prints 6 sig fig, and compare to the text in the new particle table, all the printed digits agree exactly for the mass of the ele, mu, pi0 and pi+. This is not definitive since the variables are doubles, but I'd have to figure out some code to go further in sig fig. Let me know if I have to do that.

@FNALbuild
Copy link
Collaborator

📝 The HEAD of main has changed to b220f75. Tests are now out of date.

@kutschke
Copy link
Contributor

kutschke commented Jan 6, 2022

@FNALbuild run build test

@FNALbuild
Copy link
Collaborator

⌛ The following tests have been triggered for 3ebb0ef: build (Build queue has 3 jobs)

@FNALbuild
Copy link
Collaborator

☀️ The tests passed at 3ebb0ef.

Test Result Details
merge Merged 3ebb0ef at b220f75
build (prof) Log file. Build time: 14 min 58 sec
ceSimReco Log file.
g4test_03MT Log file.
transportOnly Log file.
POT Log file.
g4study Log file.
cosmicSimReco Log file.
cosmicOffSpill Log file.
ceSteps Log file.
ceDigi Log file.
muDauSteps Log file.
ceMix Log file.
rootOverlaps Log file.
g4surfaceCheck Log file.
FIXME, TODO 〰️ TODO (14) FIXME (15) in 110 files
clang-tidy 〰️ 0 errors 0 warnings

N.B. These results were obtained from a build of this Pull Request at 3ebb0ef after being merged into the base branch at b220f75.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

Copy link
Contributor

@kutschke kutschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ray's tests show exact matches and the changes are straightforward. I am approve this and will merge.

@kutschke kutschke merged commit 61ab86e into Mu2e:main Jan 7, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants