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

Adding new class GEMDigiSimLink in 81X #13877

Merged

Conversation

mileva
Copy link
Contributor

@mileva mileva commented Mar 31, 2016

The new class GEMDigiSimLink is added. The class makes a relation between the produced GEM digis (strip and bx) and the properties of the simhits (tof, particle type, process type, ..).
An example analyzer is added also in the SimMuon/GEMDigitizer/test/ directory.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mileva for CMSSW_8_1_X.

It involves the following packages:

SimDataFormats/GEMDigiSimLink
SimMuon/GEMDigitizer

The following packages do not have a category, yet:

SimDataFormats/GEMDigiSimLink

@cmsbuild, @civanch, @mdhildreth, @davidlange6 can you please review it and eventually sign? Thanks.
@jhgoh 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

@civanch
Copy link
Contributor

civanch commented Mar 31, 2016

please test

@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@civanch
Copy link
Contributor

civanch commented Apr 2, 2016

@mileva , please make few updates in this PR, first of all, please, edit the title which is completely misleading. More important: is it really needed to use boost? in many other places it is removed.

In GEMDigiSimLinkReader.h better to use inheritance from edm::one::EDAnalyzer, because only in that case the test will be thread safe.

In runGEMDigiSimLinkReader_cfg.py you need to change file name - it is not possible to refer to private area.

…r, input file in runGEMDigiSimLinkReader_cfg.py moved to eos space
@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2016

Pull request #13877 was updated. @cmsbuild, @civanch, @mdhildreth, @davidlange6 can you please check and sign again.

@mileva
Copy link
Contributor Author

mileva commented Apr 3, 2016

Thank you for the comments!
The PR is updated following the comments of @civanch. I removed boost/cstdint.hpp from SimDataFormats/GEMDigiSimLink and the code is running well, but I see that boost/cstdint.hpp is included in all the DigiSimLinks in the SimDataFormats package.

first of all, please, edit the title which is completely misleading.

Sorry, but I am not quite sure which title to change. Is this the branch name? If yes, is it possible to change it now after the pull is requested?

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2016

Pull request #13877 was updated. @cmsbuild, @civanch, @mdhildreth, @davidlange6 can you please check and sign again.

@mileva
Copy link
Contributor Author

mileva commented Apr 6, 2016

Thank @civanch for the comments! I hope now the analyzer inherits the correct EDAnalyzer. The input file in the configuration might be produced following the instructions GEM simulation instructions. Should I include a link to the GEM simulation page?

@civanch
Copy link
Contributor

civanch commented Apr 6, 2016

@mileva , yes, please

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2016

Pull request #13877 was updated. @cmsbuild, @civanch, @mdhildreth, @davidlange6 can you please check and sign again.

@civanch
Copy link
Contributor

civanch commented Apr 6, 2016

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2016

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2016

@civanch
Copy link
Contributor

civanch commented Apr 7, 2016

+1

@dildick
Copy link
Contributor

dildick commented Apr 8, 2016

Can we collect the RPCDigiSimLink, GEMDigiSimLink and DTDigiSimLink (which appears to be defined in SimDataFormats/DigiSimLinks) in a subpackage "SimDataFormats/MuonDigiSimLink"?

@mileva
Copy link
Contributor Author

mileva commented Apr 8, 2016

@dildick I don't think so. Even it is possible, such change will give many dependencies in the other packages. Why do you need such change?

@mdhildreth
Copy link
Contributor

yes, let's not shuffle around DigiSimLink definitions at this point. Why would you want/need to do this?

@calabria
Copy link
Contributor

calabria commented May 4, 2016

any news on this PR?

@calabria
Copy link
Contributor

@civanch, @mdhildreth, @davidlange6 is the review complete for this PR?

@mdhildreth
Copy link
Contributor

+1

@davidlange6 davidlange6 merged commit f74a9ad into cms-sw:CMSSW_8_1_X May 25, 2016
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

7 participants