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

initial commit of 2016 L1T muon emulation #11504

Merged

Conversation

mulhearn
Copy link
Contributor

This is the initial batch of L1T Muon emulation for 2016 run. It includes the GMT emulator and data formats needed by GMT.

This code is unknown to standard workflows, so should have no effect.

@cmsbuild
Copy link
Contributor

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

initial commit of 2016 L1T muon emulation

It involves the following packages:

DataFormats/L1TMuon
DataFormats/L1Trigger
L1Trigger/L1TMuon

The following packages do not have a category, yet:

DataFormats/L1TMuon
L1Trigger/L1TMuon

@cmsbuild, @mulhearn can you please review it and eventually sign? Thanks.
@Martin-Grunewald 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.

@mulhearn
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@mulhearn
Copy link
Contributor Author

+1

@mulhearn
Copy link
Contributor Author

@davidlange6 late as usual but this has no effect on standard sequences and is reasonably sized for a change... please put into 76X if possible.

void GeometryTranslator::checkAndUpdateGeometry(const edm::EventSetup& es) {
const MuonGeometryRecord& geom = es.get<MuonGeometryRecord>();
unsigned long long geomid = geom.cacheIdentifier();
if( _geom_cache_id != geomid ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@Dr15Jones - is this the construct that we want to advise against ? I'm not sure I got the valid usecases of cacheIdentifier()

Copy link
Contributor

Choose a reason for hiding this comment

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

@davidlange6 It is a proper use of cacheIdentifier. However, I think the interface design makes it very easy to make a mistake. Essentially one must always be sure to call checkAndUpdateGeometry before ever calling any of the other methods. I would either pass the EventSetup in to all the calls or put the object into the EventSetup itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

Another option is to construct a GeometryTranslator by passing it the EventSetup and then just make a new one each event.

davidlange6 added a commit that referenced this pull request Oct 8, 2015
initial commit of 2016 L1T muon emulation
@davidlange6 davidlange6 merged commit ac68711 into cms-sw:CMSSW_7_6_X Oct 8, 2015
@davidlange6
Copy link
Contributor

@mulhearn - could you have a look at Chris's suggestions in a new PR?

@mulhearn
Copy link
Contributor Author

mulhearn commented Oct 9, 2015

@davidlange6 OK, will look into it.

@mulhearn mulhearn deleted the pr-l1t-muon-init-76x branch October 18, 2015 21:26
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

4 participants