search

cms-patatrack / cmssw

CMSSW fork of the Patatrack project
https://patatrack.web.cern.ch/patatrack/index.html
Apache License 2.0
2 stars 5 forks source link

Address some pixel local reco PR review comments #575

Closed makortel closed 3 years ago

makortel commented 3 years ago

PR description:

This PR addresses to some review comments of https://github.com/cms-sw/cmssw/pull/31721 (what I've done so far). I could still do some more for the parts of the code I've mostly done, therefore I opened this in draft mode (but I thought there would be some value in an open PR).

(this PR is against CMSSW_11_2_Patatrack on purpose, I'll cherry pick for a PR against cms-patatrack:patatrack_integration_9_N_pixel_local_reco "when done")

PR validation:

Code compiles.

makortel commented 3 years ago

How much do we want to review our updates to the CMSSW PR before merging? (my feeling is that the changes are easier to review here than there)

fwyzard commented 3 years ago

@makortel thank you for this. Drop a message if/when you'd like to see it tested.

makortel commented 3 years ago

Drop a message if/when you'd like to see it tested.

Now could be a good moment, thanks. I'm not planning to proceed further for now (because of time constraints).

makortel commented 3 years ago

Did the validation get stuck?

AdrianoDee commented 3 years ago

The validation failed on .502, among the others, with something like

An exception of category 'DictionaryNotFound' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=SiPixelRecHitFromSOA label='siPixelRecHitsLegacyPreSplitting'
   [2] Calling ProductRegistryHelper::addToRegistry, checking dictionaries for produced types
Exception Message:
No data dictionary found for the following classes:

  HostProduct<unsigned int[]>
  edm::Wrapper<HostProduct<unsigned int[]> >

To fix this what I found helpful was:

makortel commented 3 years ago

Thanks, so the dictionary generation failed somehow

makortel commented 3 years ago

Ok, it was pretty clear why the dictionaries were not generated. Could you try again now?

makortel commented 3 years ago

Now there is a merge conflict from merge of #577

makortel commented 3 years ago

Rebased on top of 11_2_X_Patatrack to fix the conflict (testing building now, will take a while)

fwyzard commented 3 years ago

thanks, I didn't even manage to ask ;-)

fwyzard commented 3 years ago

the error is

%MSG-w CUDAService:  (NoModuleName) 25-Nov-2020 03:20:59 CET pre-events
Failed to initialize the CUDA runtime.
Disabling the CUDAService.
%MSG

which shouldn't be related to these changes... will have a look

fwyzard commented 3 years ago 
cudaErrorSystemDriverMismatch: system has unsupported display driver / cuda driver combination
fwyzard commented 3 years ago

Validation summary

Reference release CMSSW_11_2_0_pre10 at 6c149b2963ee Development branch cms-patatrack/master at 47423b3df7e0 Testing branch cms-patatrack/master at 47423b3df7e0 with PRs:

Validation plots

/RelValTTbar_14TeV/CMSSW_11_2_0_pre7-PU_112X_mcRun3_2021_realistic_v8-v1/GEN-SIM-DIGI-RAW

/RelValZMM_14/CMSSW_11_2_0_pre7-112X_mcRun3_2021_realistic_v8-v2/GEN-SIM-DIGI-RAW

/RelValZEE_14/CMSSW_11_2_0_pre7-112X_mcRun3_2021_realistic_v8-v1/GEN-SIM-DIGI-RAW

Validation plots (CPU vs GPU)

/RelValTTbar_14TeV/CMSSW_11_2_0_pre7-PU_112X_mcRun3_2021_realistic_v8-v1/GEN-SIM-DIGI-RAW

/RelValZMM_14/CMSSW_11_2_0_pre7-112X_mcRun3_2021_realistic_v8-v2/GEN-SIM-DIGI-RAW

/RelValZEE_14/CMSSW_11_2_0_pre7-112X_mcRun3_2021_realistic_v8-v1/GEN-SIM-DIGI-RAW

Throughput plots

/EphemeralHLTPhysics1/Run2018D-v1/RAW run=323775 lumi=53

scan-136.885502.png zoom-136.885502.png scan-136.885512.png zoom-136.885512.png scan-136.885522.png zoom-136.885522.png

logs and nvprof/nvvp profiles

/RelValTTbar_14TeV/CMSSW_11_2_0_pre7-PU_112X_mcRun3_2021_realistic_v8-v1/GEN-SIM-DIGI-RAW

/RelValZMM_14/CMSSW_11_2_0_pre7-112X_mcRun3_2021_realistic_v8-v2/GEN-SIM-DIGI-RAW

/RelValZEE_14/CMSSW_11_2_0_pre7-112X_mcRun3_2021_realistic_v8-v1/GEN-SIM-DIGI-RAW

Logs

The full log is available at https://patatrack.web.cern.ch/patatrack/validation/pulls/ba84e889f4c5b4d996bf71fdb978193ac7cb6068/log .

fwyzard commented 3 years ago

Looks good to me. Shall I merge it, or do you expect further changes ?