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

Enable mkFit for initialStepPreSplitting with trackingMkFit modifier #34198

Merged
merged 2 commits into from
Jun 26, 2021

Conversation

makortel
Copy link
Contributor

PR description:

This PR extends the mkFit setup (enabled with trackingMkFit modifier, X.7 workflows) to use mkFit in initialStepPreSplitting, as mentioned in TRK POG meeting 2021-06-14.

This PR also includes a technical fix to include a header of PropagatorWithMaterial instead of its source file (I have no idea how that ended up there).

PR validation:

Workflows 11634.0, 11634.7 run.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34198/23415

  • This PR adds an extra 48KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @makortel (Matti Kortelainen) for master.

It involves the following packages:

Configuration/ProcessModifiers
RecoTracker/IterativeTracking
RecoTracker/MkFit
Validation/RecoTrack

@perrotta, @andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @silviodonato, @jfernan2, @slava77, @jpata, @qliphy, @davidlange6, @fabiocos, @rvenditti can you please review it and eventually sign? Thanks.
@fabiocos, @felicepantaleo, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @Martin-Grunewald, @wmtford, @gpetruc, @ebrondol, @mtosi, @dgulhan 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

@makortel
Copy link
Contributor Author

@cmsbuild, please test

@makortel
Copy link
Contributor Author

@cmsbuild, please abort

@makortel
Copy link
Contributor Author

test parameters:

  • workflows = 11634.7

@makortel
Copy link
Contributor Author

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-870ac7/16140/summary.html
COMMIT: 5e34756
CMSSW: CMSSW_12_0_X_2021-06-20-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34198/16140/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 8 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2785631
  • DQMHistoTests: Total failures: 12
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2785596
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.004 KiB( 37 files compared)
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 160 log files, 37 edm output root files, 38 DQM output files
  • TriggerResults: no differences found

Copy link
Contributor

@jpata jpata left a comment

Choose a reason for hiding this comment

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

@mtosi @vmariani please let us know if you have additional comments/concerns.

Comment on lines 128 to 136
from Configuration.ProcessModifiers.trackingMkFitInitialStepPreSplitting_cff import trackingMkFitInitialStepPreSplitting
from RecoTracker.MkFit.mkFitGeometryESProducer_cfi import mkFitGeometryESProducer
import RecoTracker.MkFit.mkFitSiPixelHitConverter_cfi as mkFitSiPixelHitConverter_cfi
import RecoTracker.MkFit.mkFitSiStripHitConverter_cfi as mkFitSiStripHitConverter_cfi
import RecoTracker.MkFit.mkFitEventOfHitsProducer_cfi as mkFitEventOfHitsProducer_cfi
import RecoTracker.MkFit.mkFitSeedConverter_cfi as mkFitSeedConverter_cfi
import RecoTracker.MkFit.mkFitIterationConfigESProducer_cfi as mkFitIterationConfigESProducer_cfi
import RecoTracker.MkFit.mkFitProducer_cfi as mkFitProducer_cfi
import RecoTracker.MkFit.mkFitOutputConverter_cfi as mkFitOutputConverter_cfi
Copy link
Contributor

Choose a reason for hiding this comment

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

sorry if I miss something, but should/could these be hidden from re-exporting (prefixed with _)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Certainly.

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 changed only the InitialStepPreSplitting_cff.py that was modified in this PR. If you wish, I can change the other iterations where mkFit can be enabled accordingly as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks! maybe the other iterations can be changed in a (technical) follow-up?

@makortel makortel force-pushed the mkFitInitialStepPreSplitting branch from 5e34756 to eac614f Compare June 22, 2021 13:44
@cmsbuild
Copy link
Contributor

Pull request #34198 was updated. @perrotta, @andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @silviodonato, @jfernan2, @slava77, @jpata, @qliphy, @davidlange6, @fabiocos, @rvenditti can you please check and sign again.

@makortel
Copy link
Contributor Author

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-870ac7/16164/summary.html
COMMIT: eac614f
CMSSW: CMSSW_12_0_X_2021-06-22-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34198/16164/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2785631
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2785608
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
  • Checked 160 log files, 37 edm output root files, 38 DQM output files
  • TriggerResults: no differences found

@jpata
Copy link
Contributor

jpata commented Jun 25, 2021

+reconstruction

  • mkFit in initialStepPreSplitting with a modifier
  • the mkFit workflow 11634.7 runs
  • no reco differences in standard workflows

@jfernan2
Copy link
Contributor

+1

@qliphy
Copy link
Contributor

qliphy commented Jun 26, 2021

+1

@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 be automatically merged.

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.

5 participants