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

[HGCAL] SimTracksters from Caloparticles #33638

Merged
merged 7 commits into from May 10, 2021

Conversation

felicepantaleo
Copy link
Contributor

PR description:

This PR improves SimTracksters by:

  • taking into account the correct fraction of the energy of the layer cluster for the calculation of the energy of the trackster and its PCA (data format change was needed)
  • using the caloparticle as a source for layercluster if the caloparticle crossed the CALO volume boundary, otherwise use the simclusters.
  • I expect changes in the number of simtracksters (I expect a smaller number of them)

PR validation:

validated using 23293.0

@felicepantaleo
Copy link
Contributor Author

@cmsbuild please test

@@ -153,7 +153,7 @@ namespace ticl {
// The vertices of the DAG are the indices of the
// 2d objects in the global collection
std::vector<unsigned int> vertices_;
std::vector<uint8_t> vertex_multiplicity_;
std::vector<float> vertex_multiplicity_;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious if the schema evolution handles this appropriately.
@felicepantaleo please clarify/confirm that tracksters from the old files are still correctly readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could run the trackster validation on old files with this branch.

@cmsbuild
Copy link
Contributor

cmsbuild commented May 5, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33638/22508

@cmsbuild
Copy link
Contributor

cmsbuild commented May 5, 2021

A new Pull Request was created by @felicepantaleo (Felice Pantaleo) for master.

It involves the following packages:

DataFormats/HGCalReco
RecoHGCal/TICL

@perrotta, @jpata, @srimanob, @kpedro88, @slava77 can you please review it and eventually sign? Thanks.
@rovere, @apsallid, @sobhatta, @lecriste, @hatakeyamak, @ebrondol, @clelange this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented May 5, 2021

-1

Failed Tests: RelVals
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-81c9a3/14889/summary.html
COMMIT: 5a23cbc
CMSSW: CMSSW_12_0_X_2021-05-04-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33638/14889/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

----- Begin Fatal Exception 05-May-2021 18:37:02 CEST-----------------------
An exception of category 'ProductNotFound' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 1 stream: 0
   [1] Running path 'validation_step9'
   [2] Prefetching for module HGCalValidator/'hgcalValidator'
   [3] Calling method for module TrackstersFromSimClustersProducer/'ticlSimTracksters'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for type: std::vector<CaloParticle>
Looking for module label: mix
Looking for productInstanceName: MergedCaloTruth

   Additional Info:
      [a] If you wish to continue processing events after a ProductNotFound exception,
add "SkipEvent = cms.untracked.vstring('ProductNotFound')" to the "options" PSet in the configuration.

----- End Fatal Exception -------------------------------------------------

@kpedro88
Copy link
Contributor

kpedro88 commented May 5, 2021

@felicepantaleo you need to propagate the premixing modification to the new caloparticles parameter:

simclusters = "mixData:MergedCaloTruth"

@felicepantaleo
Copy link
Contributor Author

@cmsbuild please test

@felicepantaleo
Copy link
Contributor Author

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 6, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33638/22520

@cmsbuild
Copy link
Contributor

cmsbuild commented May 6, 2021

Pull request #33638 was updated. @perrotta, @jpata, @srimanob, @kpedro88, @slava77 can you please check and sign again.

@felicepantaleo
Copy link
Contributor Author

ok, I think I am done with it...

@cmsbuild
Copy link
Contributor

cmsbuild commented May 6, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-81c9a3/14908/summary.html
COMMIT: 5ed978a
CMSSW: CMSSW_12_0_X_2021-05-05-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33638/14908/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

The workflows 140.53 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 1263 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2662646
  • DQMHistoTests: Total failures: 8395
  • DQMHistoTests: Total nulls: 19
  • DQMHistoTests: Total successes: 2654210
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -45.703 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 140.53 ): -44.531 KiB Hcal/DigiRunHarvesting
  • DQMHistoSizes: changed ( 140.53 ): -1.172 KiB RPC/DCSInfo
  • Checked 155 log files, 37 edm output root files, 37 DQM output files
  • TriggerResults: no differences found

@jpata
Copy link
Contributor

jpata commented May 7, 2021

+reconstruction

  • no unexpected/relevant changes

@srimanob
Copy link
Contributor

srimanob commented May 7, 2021

Hi @felicepantaleo

Is this something you expect?
https://tinyurl.com/yf2sc7ln

Screen Shot 2564-05-07 at 17 57 01

@felicepantaleo
Copy link
Contributor Author

@srimanob yes, definitely

@srimanob
Copy link
Contributor

+Upgrade

This PR is expected to improve SimTracksters. Changes have been seen, and they are expected as confirmed by HGCAL.

@cmsbuild
Copy link
Contributor

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)

@qliphy
Copy link
Contributor

qliphy commented May 10, 2021

+1

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

Successfully merging this pull request may close these issues.

None yet

7 participants