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.

Sign up for GitHub

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

Jump to bottom

Change FastSim vertex smearing to FullSim approach #5096

Merged
merged 4 commits into from Sep 9, 2014
Merged

Change FastSim vertex smearing to FullSim approach #5096

merged 4 commits into from Sep 9, 2014

Conversation

lveldere
Copy link
Contributor

First step in streamlining the FastSim configuration to FullSim
Up till know FastSim smeared the primary vertex of the gen event inside the FastSim Producer, with dedicated vertex smearing classes.
Now this is done by a VtxGenerator, as for FullSim, before the FastSim producer is called.
The PileUpProducer still uses the dedicated vertex smearing classes.
These classes will be moved to the PileUpProducer package in a next commit / pr
This allows to simplify the FastSim code, and the FastSim related python code in the ConfigBuilder.

First step in streamlining the FastSim configuration to FullSim
2b79879 
Up till know FastSim smeared the primary vertex of the gen event inside the FastSim Producer, with dedicated vertex smearing classes.
Now this is done by a VtxGenerator, as for FullSim, before the FastSim producer is called.
The PileUpProducer still uses the dedicated vertex smearing classes.
These classes will be moved to the PileUpProducer package in a next commit / pr
This allows to simplify the FastSim code, and the FastSim related python code in the ConfigBuilder.
@lveldere
Copy link
Contributor Author

hit the button too early

@lveldere lveldere closed this Aug 28, 2014
@lveldere
Copy link
Contributor Author

After basic validation I re-open this pr.
Statistical differences expected everywhere.
Only systematic differences expected in vertices of GenParticleCollection.
(fixed a bug)

@lveldere lveldere reopened this Aug 29, 2014
@cmsbuild
Copy link
Contributor

A new Pull Request was created by @lveldere for CMSSW_7_2_X.

Change FastSim vertex smearing to FullSim approach

It involves the following packages:

Configuration/Applications
FastSimulation/Configuration
FastSimulation/Event
FastSimulation/EventProducer
FastSimulation/PileUpProducer
IOMC/EventVertexGenerators

@civanch, @nclopezo, @lveldere, @vlimant, @mdhildreth, @cmsbuild, @franzoni, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.

@cmsbuild
Copy link
Contributor

-1
Tested at: 2b79879
When I ran the RelVals I found an error in the following worklfows:
8.0 step1

runTheMatrix-results/8.0_BeamHalo+BeamHalo+DIGICOS+RECOCOS+ALCABH+HARVESTCOS/step1_BeamHalo+BeamHalo+DIGICOS+RECOCOS+ALCABH+HARVESTCOS.log

9.0 step1

runTheMatrix-results/9.0_Higgs200ChargedTaus+Higgs200ChargedTaus+DIGI+RECO+HARVEST/step1_Higgs200ChargedTaus+Higgs200ChargedTaus+DIGI+RECO+HARVEST.log

25.0 step1

runTheMatrix-results/25.0_TTbar+TTbar+DIGI+RECO+HARVEST+ALCATT/step1_TTbar+TTbar+DIGI+RECO+HARVEST+ALCATT.log

1306.0 step1

runTheMatrix-results/1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15/step1_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15.log

25202.0 step1

runTheMatrix-results/25202.0_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVEST+MINIAODMC/step1_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVEST+MINIAODMC.log

101.0 step1

runTheMatrix-results/101.0_SingleElectronE120EHCAL+SingleElectronE120EHCAL/step1_SingleElectronE120EHCAL+SingleElectronE120EHCAL.log

401.0 step1

runTheMatrix-results/401.0_TTbarNewMix+TTbarFSPU2+HARVESTFS/step1_TTbarNewMix+TTbarFSPU2+HARVESTFS.log

4.22 step3

runTheMatrix-results/4.22_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC/step3_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC.log

5.1 step2

runTheMatrix-results/5.1_TTbar+TTbarFS+HARVESTFS/step2_TTbar+TTbarFS+HARVESTFS.log

1001.0 step3

runTheMatrix-results/1001.0_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVD/step3_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVD.log

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5096/737/summary.html

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 1, 2014

-1
Tested at: 2b79879
When I ran the RelVals I found an error in the following worklfows:
8.0 step1

runTheMatrix-results/8.0_BeamHalo+BeamHalo+DIGICOS+RECOCOS+ALCABH+HARVESTCOS/step1_BeamHalo+BeamHalo+DIGICOS+RECOCOS+ALCABH+HARVESTCOS.log

9.0 step1

runTheMatrix-results/9.0_Higgs200ChargedTaus+Higgs200ChargedTaus+DIGI+RECO+HARVEST/step1_Higgs200ChargedTaus+Higgs200ChargedTaus+DIGI+RECO+HARVEST.log

25.0 step1

runTheMatrix-results/25.0_TTbar+TTbar+DIGI+RECO+HARVEST+ALCATT/step1_TTbar+TTbar+DIGI+RECO+HARVEST+ALCATT.log

1306.0 step1

runTheMatrix-results/1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15/step1_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15.log

101.0 step1

runTheMatrix-results/101.0_SingleElectronE120EHCAL+SingleElectronE120EHCAL/step1_SingleElectronE120EHCAL+SingleElectronE120EHCAL.log

25202.0 step1

runTheMatrix-results/25202.0_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVEST+MINIAODMC/step1_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVEST+MINIAODMC.log

401.0 step1

runTheMatrix-results/401.0_TTbarNewMix+TTbarFSPU2+HARVESTFS/step1_TTbarNewMix+TTbarFSPU2+HARVESTFS.log

4.22 step3

runTheMatrix-results/4.22_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC/step3_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC.log

5.1 step2

runTheMatrix-results/5.1_TTbar+TTbarFS+HARVESTFS/step2_TTbar+TTbarFS+HARVESTFS.log

1001.0 step3

runTheMatrix-results/1001.0_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVD/step3_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVD.log

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5096/747/summary.html

fixed small issues that caused bugs in standard tests
e075bac 
tested with workflows 1325.0 (fullsim) and 135.1 (fastsim)
@lveldere
Copy link
Contributor Author

lveldere commented Sep 1, 2014

Can the tests be resumed?
All is supposed to work now.
Lukas

@lveldere
Copy link
Contributor Author

lveldere commented Sep 4, 2014

@franzoni

Hi Giovanni

Yes, 400 and 401 are the same now.
Before my change, workflow 401 used a pu mixing scheme that is no longer supported,
and from this pr on even doesn't work anymore. (actually it was never supported)
The physics point of view doesn't really matter here:
the pu mechanism that is currently behind PUFS2 does not work anymore and will not be revived.

Workflow 401 is included in the jenkins tests.
To avoid failures of the jenkins tests I simply redefined the workflow.
(If my pr gets merged w/o this change, it will cause all later prs to fail tests)

This is a temporary solution: I would like to keep the "slot" open:
I am currently working on a next pr, which will enable to configure FastSim with the new digi-reco mixing through cmsDriver, and I think it makes sense to replace PUFS2 with that new mixing scheme.

I see in my '401-change' no reason to keep this pr from being approved.
However, it can be another discussion whether 401 should stay in the Jenkins tests or not,
and whether PUFS2 can be used to accommodate the new digi-reco mixing for FastSim.

Cheers

Lukas

@lveldere
Copy link
Contributor Author

lveldere commented Sep 8, 2014

operations, what's keeping us from moving forward?

@lveldere
Copy link
Contributor Author

lveldere commented Sep 8, 2014

reminder

@lveldere
Copy link
Contributor Author

lveldere commented Sep 8, 2014

reminder for operations

@lveldere
Copy link
Contributor Author

lveldere commented Sep 9, 2014

@franzoni

Would you mind signing?
I think we reached agreement.

BTW, in the meanwhile I verified explicitly that

  • cfg dumps from FullSim runTheMatrix workflows are not affected by my changes
  • cfg dumps from FastSim runTheMatrix workflows are only affected where it should be

@franzoni
Copy link

franzoni commented Sep 9, 2014

+1

  • Lukas and I agreed verbally that wf 401 will be temporarily set identical to 400
  • Validation can continue using the postLs1 fast sim w/pileup samples
  • Lukas to follow up with Giulio/release integration to change the bot so that wf 401 is replaced (e.g. by 25401) among the restricted pool which is run to test PR's/

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2014

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_2_X IBs unless changes (tests are also fine).

@lveldere
Copy link
Contributor Author

lveldere commented Sep 9, 2014

thanks Giovanni

On Tue, Sep 9, 2014 at 5:39 PM, Giovanni Franzoni notifications@github.com
wrote:

+1

  • Lukas and I agreed verbally that wf 401 will be temporarily set
    identical to 400
  • Validation can continue using the postLs1 fast sim w/pileup samples
  • Lukas to follow up with Giulio/release integration to change the bot
    so that wf 401 is replaced (e.g. by 25401) among the restricted pool which
    is run to test PR's/

—
Reply to this email directly or view it on GitHub
#5096 (comment).

davidlange6 added a commit that referenced this pull request Sep 9, 2014
@davidlange6
Merge pull request #5096 from lveldere/fastsim-vertex-smearing-ala-fu…
75d2635 
…llsim

Change FastSim vertex smearing to FullSim approach
@davidlange6 davidlange6 merged commit 75d2635 into cms-sw:CMSSW_7_2_X Sep 9, 2014
@cmsbuild cmsbuild mentioned this pull request Sep 10, 2014
Merged
davidlange6 added a commit that referenced this pull request Sep 11, 2014
@davidlange6
Merge pull request #5263 from lveldere/fastsim-puproducer-complete-cfg
4d12934 
fixed small issue introduced in #5096
@lveldere lveldere deleted the fastsim-vertex-smearing-ala-fullsim branch January 20, 2015 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
comparison-pending fastsim-approved fully-signed operations-approved tests-approved
Projects
None yet
Milestone
Next CMSSW_7_2_X
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@lveldere @cmsbuild @civanch @franzoni @davidlange6