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

Update DeepCore training for Run-3 #38169

Closed
wants to merge 1 commit into from

Conversation

mmusich
Copy link
Contributor

@mmusich mmusich commented Jun 1, 2022

PR description:

This PR is a companion to cms-data/RecoTracker-TkSeedGenerator#3 which updates the training for DeepCore using RUn-3 MC samples (the original training was done on 2017 MC).
This should reflect the updates in the Pixel conditions and tracking strategy with respect to Run2.

PR validation:

None.

if this PR is a backport please specify the original PR and why you need to backport that PR:

N/A

cc:
@bouchamaouihichem

@mmusich
Copy link
Contributor Author

mmusich commented Jun 1, 2022

test parameters:

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2022

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38169/30318

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2022

A new Pull Request was created by @mmusich (Marco Musich) for master.

It involves the following packages:

  • RecoTracker/IterativeTracking (reconstruction)

@jpata, @cmsbuild, @clacaputo, @slava77 can you please review it and eventually sign? Thanks.
@felicepantaleo, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @ebrondol, @gpetruc, @mmusich, @mtosi, @dgulhan this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@mmusich
Copy link
Contributor Author

mmusich commented Jun 1, 2022

please test

@mmusich
Copy link
Contributor Author

mmusich commented Jun 1, 2022

@smuzaffar the tests of this PR are building cmssw since 6 hours now, is it normal?

@smuzaffar
Copy link
Contributor

@mmusich , this is because we updated root which is causing a lot of externals and cmssw packages to rebuild. There is a newer IB we updated root available. I wopuld suggest to restart the tests to make use of new IB

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 2, 2022

-1

Failed Tests: UnitTests RelVals
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bf98ab/25161/summary.html
COMMIT: cc564a0
CMSSW: CMSSW_12_5_X_2022-06-01-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38169/25161/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bf98ab/25161/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bf98ab/25161/git-merge-result

Unit Tests

I found errors in the following unit tests:

---> test SiStripDAQ_O2O_test had ERRORS

RelVals

----- Begin Fatal Exception 02-Jun-2022 01:04:43 CEST-----------------------
An exception of category 'InvalidGraphDef' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=DeepCoreSeedGenerator label='jetCoreRegionalStepSeedsBarrel'
Exception Message:
error while loading graphDef from '/cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38169/25161/CMSSW_12_5_X_2022-06-01-1100/external/el8_amd64_gcc10/data/RecoTracker/TkSeedGenerator/data/DeepCore/DeepCoreSeedGenerator_TrainedModel_barrel_2022.pb': Data loss: Can't parse /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38169/25161/CMSSW_12_5_X_2022-06-01-1100/external/el8_amd64_gcc10/data/RecoTracker/TkSeedGenerator/data/DeepCore/DeepCoreSeedGenerator_TrainedModel_barrel_2022.pb as binary proto
----- End Fatal Exception -------------------------------------------------

@jpata
Copy link
Contributor

jpata commented Jun 3, 2022

@mmusich @bouchamaouihichem I understand that before proceeding with this, the plan is to fix the .pb file, i.e. this PR is "do not merge".

just in case, maybe @riga @yongbinfeng know what could have gone wrong here.

@mmusich
Copy link
Contributor Author

mmusich commented Jun 3, 2022

I understand that before proceeding with this, the plan is to fix the .pb file, i.e. this PR is "do not merge".

well, eventually the PR is to be merged once the pb file is fixed. This was only to expose the problem.

@jpata
Copy link
Contributor

jpata commented Aug 1, 2022

-reconstruction

  • will look at it again once it's updated.

@mmusich
Copy link
Contributor Author

mmusich commented Aug 1, 2022

@jpata please don't reject this PR. There is no plan to update the code here, it just needs an update of the external to be merged.

@jpata
Copy link
Contributor

jpata commented Aug 1, 2022

Right, fair point. It's not rejected as such - once cms-data/RecoTracker-TkSeedGenerator#2 is updated, I will remove the -sig and test again.

@mmusich mmusich force-pushed the updateRun3DeepCoreTraining branch from cc564a0 to edd48ca Compare September 19, 2023 06:53
@mmusich mmusich changed the title use run3_common modifier to update DeepCore training for Run-3 Update DeepCore training for Run-3 Sep 19, 2023
@mmusich
Copy link
Contributor Author

mmusich commented Sep 19, 2023

test parameters:

@mmusich mmusich mentioned this pull request Sep 19, 2023
@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38169/36938

  • This PR adds an extra 20KB to repository

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

@cmsbuild
Copy link
Contributor

Pull request #38169 was updated. @cmsbuild, @jfernan2, @mandrenguyen can you please check and sign again.

@mmusich
Copy link
Contributor Author

mmusich commented Sep 19, 2023

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bf98ab/34808/summary.html
COMMIT: edd48ca
CMSSW: CMSSW_13_3_X_2023-09-18-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38169/34808/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially removed 2 lines from the logs
  • Reco comparison results: 1813 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3461608
  • DQMHistoTests: Total failures: 644
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3460942
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
  • Checked 218 log files, 172 edm output root files, 51 DQM output files
  • TriggerResults: no differences found

@mmusich
Copy link
Contributor Author

mmusich commented Sep 25, 2023

Closing in favor of #42851

@mmusich mmusich closed this Sep 25, 2023
@mmusich mmusich deleted the updateRun3DeepCoreTraining branch September 25, 2023 08:32
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