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 OfflinePV and Beamspot in trackingLowPU era #14150

Merged

Conversation

rovere
Copy link
Contributor

@rovere rovere commented Apr 19, 2016

The trackingLowPU era has been cooked to perform tracking
also in the B=0 case, when the Pixel detector could be
excluded from data taking. Tracking was already working, but
the configuration for primary vertices and beamspot
calculation had to be tuned to properly work also in the
case in which Pixels are out of data taking. The code has
been successfully tested on Run 269207.

The trackingLowPU era has been cooked to perform tracking
also in the B=0 case, when the Pixel detector could be
excluded from data taking. Tracking was already working, but
the configuration for primary vertices and beamspot
calculation had to be tuned to properly work also in the
case in which Pixels are out of data taking. The code has
been successfully tested on Run 269207.
@cmsbuild
Copy link
Contributor

A new Pull Request was created by @rovere (Marco Rovere) for CMSSW_8_0_X.

It involves the following packages:

DQM/BeamMonitor
RecoVertex/PrimaryVertexProducer

@cvuosalo, @cmsbuild, @deguio, @slava77, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks.
@makortel, @cerati, @GiacomoSguazzoni, @dgulhan, @VinInn this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@slava77
Copy link
Contributor

slava77 commented Apr 19, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/12496/console

@slava77
Copy link
Contributor

slava77 commented Apr 19, 2016

the set of changes with whitespaces skipped is more clear
https://github.com/cms-sw/cmssw/pull/14150/files?w=1

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor

slava77 commented Apr 20, 2016

... the meaning of "trackingLowPU" is becoming more and more "trackingNoPixel"
Or is it really, "trackingNoPixelB0T"?

@rovere
Copy link
Contributor Author

rovere commented Apr 20, 2016

trackingNoPixelB0T, since with 3.8T we can safely use the ordinary tracking, even w/o pixels.

@makortel
Copy link
Contributor

Or "trackingB0T"? (I'm just thinking that for "trackingNoPixel*" we could, in principle, just remove all pixel-seeded iterations, which, on the other hand, is probably not what we want to do for all 0T reconstruction).

@rovere
Copy link
Contributor Author

rovere commented Apr 20, 2016

trackingB0T even better

@slava77
Copy link
Contributor

slava77 commented Apr 20, 2016

On 4/20/16 11:33 AM, Marco Rovere wrote:

trackingB0T even better

no, this will not work, for a decent pileup (>10) we will run regular
tracking

So "NoPixel" should be in the name

@VinInn
Copy link
Contributor

VinInn commented Apr 20, 2016

This is NOT noPixel, LowPileup or B=0 is just RunI-like config
the tiny cut is needed to protect against 0 charge clusters independent of pixel, pileup, field
so just call it RunIlike please no explicit mention of field pixel, pileup

@slava77
Copy link
Contributor

slava77 commented Apr 20, 2016

On 4/20/16 1:14 PM, Vincenzo Innocente wrote:

This is NOT noPixel, LowPileup or B=0 is just RunI-like config
the tiny cut is needed to protect against 0 charge clusters independent
of pixel, pileup, field
so just call it RunIlike please no explicit mention of field pixel, pileup

Is it worth supporting Run1-like long-term in this current form
(different modules and different cuts in the sequence)?

For operations we need more practical setups for the detector setup,
instead of a time machine option,
which by itself is pretty complex and just happens to work for no-pixel
B=0 T.
(well, that's unless there are arguments that this Run1-like
configuration is optimized for B=0T without pixel
from physics perspective, fake vs efficiency).


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#14150 (comment)

@rovere
Copy link
Contributor Author

rovere commented Apr 22, 2016

@cvuosalo:
I tested this PR with the following command:

python $CMSSW_BASE/src/Configuration/DataProcessing/test/RunPromptReco.py --scenario ppEra_Run2_2016_trackingLowPU --reco --dqmio --global-tag 80X_dataRun2_Prompt_v4 --lfn=file:/data/rovere/2016data/MinimumBias/269207/74E35575-E1FF-E511-B11A-02163E01197B.root

Of course you have to change the input filename and use any from run 269207. I got rid of all AlCa* stuff to make it run faster.

Let me know if you need anything more.
By the way, while I received your message in my inbox, somehow I was not able to find it in here...

@deguio
Copy link
Contributor

deguio commented Apr 22, 2016

+1

@cvuosalo
Copy link
Contributor

cvuosalo commented Apr 26, 2016

@rovere: I tested 120,000 events against baseline CMSSW_8_0_4 and found no effect from this PR. What differences should I expect to find?
Here are the commands I used on a file from run 269207:

python $CMSSW_RELEASE_BASE/src/Configuration/DataProcessing/test/RunPromptReco.py  --scenario ppEra_Run2_2016_trackingLowPU --global-tag  80X_dataRun2_Prompt_v4 --lfn  /store/data/Commissioning2016/MinimumBias/RAW/v1/000/264/232/00000/2A2C0128-81D1-E511-BBA4-02163E014520.root --reco --miniaod --aod --dqmio

cmsDriver.py step4  --filetype DQM --data --conditions 80X_dataRun2_Prompt_v4 -s HARVESTING:@standardDQM+@miniAODDQM --scenario pp --era Run2_2016_trackingLowPU -n -1 --filein file:output_inDQMIO.root --fileout file:step4.root  |& tee step4.log

@rovere
Copy link
Contributor Author

rovere commented Apr 26, 2016

@cvuosalo
your lfn is /store/data/Commissioning2016/MinimumBias/RAW/v1/000/264/232/00000/......root, so does not seem to belong to run 269207??
On a recent quiet-beam run, w/o pixel, with 0T and using lowPU scenario you should be able to see a filled vertex collection and a correct beamspot.

I'll try to reprocess the run and report back in this thread.

@cvuosalo
Copy link
Contributor

@rovere: Thanks for spotting my error. I had the correct dataset but the file was from the wrong run. I obtained a correct file for run 269207, and now the test results do show differences due to the correct beam spot and primary vertices. Details in the next message.

@cvuosalo
Copy link
Contributor

A test was performed using run 269207, which had a quiet beam and no Pixel, with 500 events against baseline CMSSW_8_0_4. The command used was:

python $CMSSW_RELEASE_BASE/src/Configuration/DataProcessing/test/RunPromptReco.py  --scenario ppEra_Run2_2016_trackingLowPU --global-tag  80X_dataRun2_Prompt_v4 --lfn /store/data/Commissioning2016/MinimumBias/RAW/v1/000/269/207/00000/EECA49D4-E1FF-E511-B110-02163E0135FC.root --reco --miniaod --aod --dqmio

The results show the effects of the corrected beam spot and filled primary vertex collections, and no other problems. Some example plots follow.

pvz

offlinepvcoll
numofflinevert
pvxpoc

pvzpoc
hitstec
met

@cvuosalo
Copy link
Contributor

+1

For #14150 3ffbee4

Enabling offline vertices and correct beam spot calculation for the B=0T and no-Pixel situation. There should be no change in standard workflows.

The code changes are satisfactory, and Jenkins tests against baseline CMSSW_8_0_X_2016-04-19-1100 show no significant differences, as expected. A test of a special run that had quiet beam and no Pixel shows the correct behavior as discussed above. Without these special circumstances, this PR causes no changes.

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@slava77
Copy link
Contributor

slava77 commented May 2, 2016

There is no 81X version, right?

@rovere
Copy link
Contributor Author

rovere commented May 3, 2016

Ciao @slava77,
not there is no 81X corresponding PR. Can we have this forward ported/cherry-picked automagically or should I take care of it?

@slava77
Copy link
Contributor

slava77 commented May 3, 2016

Ciao Marco,
The policy is to integrate changes in a development release first, which is 81X.
You are now relying either on the policy or the causality to be violated.

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 8a0105f into cms-sw:CMSSW_8_0_X May 11, 2016
@rovere rovere deleted the enableBeamSpotIn_trackingLowPU_era branch October 3, 2023 07:42
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

8 participants