-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Calo boundary Information #33056
Calo boundary Information #33056
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33056/21361
|
A new Pull Request was created by @rovere (Marco Rovere) for master. It involves the following packages: SimG4CMS/Calo @cmsbuild, @civanch, @mdhildreth can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild please test |
FYI @cseez |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1c36a6/13249/summary.html Comparison SummarySummary:
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33056/21372
|
Pull request #33056 was updated. @cmsbuild, @civanch, @mdhildreth can you please check and sign again. |
@cmsbuild please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1c36a6/13261/summary.html Comparison SummarySummary:
|
DelivLuminosity = cms.double(5000.) | ||
) | ||
|
||
common_MCtruth = cms.PSet( | ||
DoFineCalo = cms.bool(False), | ||
SaveCaloBoundaryInformation = cms.bool(False), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rovere , I am sorry, can you imagine that somebody want DoFineCalo=True and SaveCaloBoundaryInformation=False?
What is the use case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ciao @civanch
thanks for looking into that!
The use case here is that we would like to have the information at the caloBoundary without requesting the full complexity of tracing everything within the fine-calo volumes.
The two flags, in this sense, are complementary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rovere , why we see differences in Phase-2 WFs? I would expect regression for this PR. DD4Hep WF may be fine so far.
ciao @civanch
the flag is set to false
for all workflows but for Phase2
ones.
The changes are mostly coming from SimTracks that are now saved if they have the crossedBoundary flag set. I found this useful to correctly assign hits to SimTrack and SimClusters.
The extent of the changes is rather limited and of limited impact on the high level Validation.
The DD4HEP workflows shows many differences, but I'm not sure where they are coming from, but you seem to suggest that is fine, right?
@rovere , why we see differences in Phase-2 WFs? I would expect regression for this PR. DD4Hep WF may be fine so far. |
+1 |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1
|
PR description:
This PR adds the information at CALO boundary to the default configuration.
PR validation:
runTheMatrix limited.