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

use fastSim era to apply FastSim mods on TrackValidation_cff #10922

Merged
merged 6 commits into from
Aug 27, 2015
Merged

use fastSim era to apply FastSim mods on TrackValidation_cff #10922

merged 6 commits into from
Aug 27, 2015

Conversation

lveldere
Copy link
Contributor

After a discussion in the simulation meeting of Aug 21,
a new era 'fastSim' was introduced by Mark Grimes #10906 .
The plan is to use this fastSim era to apply FastSim mods on the configuration
ofValidation sequences and modules.

This pull request uses the fastSim era to apply FastSim mods on the configuration of the Tracking Validation.

To minimise the number of FastSim mods,
some copy pasted psets were replaced with clones in the file
Validation/RecoTrack/python/TrackingParticleSelectionsForEfficiency_cff.py

In addition, an obsolete, outdated cfg file is removed:
FastSimulation/Validation/python/trackingParticlesFastSim_cfi.py

@makortel
please let me know if you have any suggestions / questions

@deguio and other DQM responsibles
Does this need a discussion between the FastSim and the DQM group?
The plan would be to gradually migrate the whole validation sequence.

@lveldere
Copy link
Contributor Author

please test

@cmsbuild cmsbuild added this to the Next CMSSW_7_6_X milestone Aug 24, 2015
@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

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

use fastSim era to apply FastSim mods on TrackValidation_cff

It involves the following packages:

Configuration/Applications
Configuration/StandardSequences
FastSimulation/Validation
SimTracker/TrackAssociatorProducers
Validation/RecoTrack

@civanch, @lveldere, @danduggan, @ssekmen, @mdhildreth, @cmsbuild, @franzoni, @deguio, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @makortel, @istaslis, @GiacomoSguazzoni, @rovere, @VinInn, @Martin-Grunewald, @matt-komm, @wmtford, @cerati, @threus, @dgulhan 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.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

lip = cms.double(30.0),
tip = cms.double(3.5)
)
# TODO: clone all these guys from the first one
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks :) But isn't the comment obsolete? (i.e. the todo is already done)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep :-)

On Mon, Aug 24, 2015 at 2:52 PM, Matti Kortelainen <notifications@github.com

wrote:

In
Validation/RecoTrack/python/TrackingParticleSelectionsForEfficiency_cff.py
#10922 (comment):

-)

-TpSelectorForEfficiencyVsVTXZBlock = cms.PSet(

  • chargedOnly = cms.bool(True),
  • pdgId = cms.vint32(),
  • signalOnly = cms.bool(True),
  • intimeOnly = cms.bool(False),
  • stableOnly = cms.bool(False),
  • minRapidity = cms.double(-2.5),
  • minHit = cms.int32(0),
  • ptMin = cms.double(0.9),
  • maxRapidity = cms.double(2.5),
  • lip = cms.double(30.0),
  • tip = cms.double(3.5)
    -)
    +# TODO: clone all these guys from the first one

Thanks :) But isn't the comment obsolete? (i.e. the todo is already done)


Reply to this email directly or view it on GitHub
https://github.com/cms-sw/cmssw/pull/10922/files#r37747172.

@makortel
Copy link
Contributor

@lveldere Looks good to me. I only want to note that now also the three other MTV instances (trackValidatorFromPV, trackValidatorFromPVAllTP, trackValidatorAllTPEffic) will be run for FastSim. Probably that is fine (or even desired), but I just wanted to note it explicitly.

@lveldere
Copy link
Contributor Author

@makortel
This is indeed by intention.
Things run without crashes, but I did not check whether the output of these new instances makes sense.

@cmsbuild
Copy link
Contributor

Pull request #10922 was updated. @civanch, @lveldere, @danduggan, @ssekmen, @mdhildreth, @cmsbuild, @franzoni, @deguio, @davidlange6 can you please check and sign again.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@lveldere
Copy link
Contributor Author

+1

comparisons look fine

@civanch
Copy link
Contributor

civanch commented Aug 24, 2015

+1

@lveldere
Copy link
Contributor Author

@danduggan @franzoni @deguio @davidlange6

Would you mind reviewing and approving for operations and DQM?
If any further discussion is needed, please inform me.
Cheers
Lukas

@danduggan
Copy link
Contributor

+1

@danduggan
Copy link
Contributor

@lveldere I've approved this, but in terms of a full scale migration for the fastsim validation sequence, it would be good to have this discussion at our next DQM/pdmv meeting (sept 1st).

@lveldere
Copy link
Contributor Author

@danduggan

okay, great.
I'll prepare a couple of slides for Tuesday,
to initiate the discussion.

On Wed, Aug 26, 2015 at 10:16 AM, danduggan notifications@github.com
wrote:

@lveldere https://github.com/lveldere I've approved this, but in terms
of a full scale migration for the fastsim validation sequence, it would be
good to have this discussion at our next DQM/pdmv meeting (sept 1st).


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

davidlange6 added a commit that referenced this pull request Aug 27, 2015
…imEra

use fastSim era to apply FastSim mods on TrackValidation_cff
@davidlange6 davidlange6 merged commit c107ac9 into cms-sw:CMSSW_7_6_X Aug 27, 2015
@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 1, 2015

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

6 participants