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

Primitive RPC trigger #26967

Merged
merged 11 commits into from Jul 9, 2019
Merged

Primitive RPC trigger #26967

merged 11 commits into from Jul 9, 2019

Conversation

maseguracern
Copy link
Contributor

This is the first version of the RPCTriggerPrimitives for the RPC detector. Which uses as an input the RPCDigis as an input and the RPCRecHits (New collection) as an output. The output of this module is an edm branch named RPCPrimitivesDigis, following the RPCRecHit format already committed in CMSSW_10_6_0. We apply the cluster size cut and emulate max two clusters per link board. The module can be tuned by the parameters LinkBoardCut and ClusterSizeCut for phaseII.

People interested: @maseguracern @BrieucF @abrinke1

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

mrodozov added a commit to cms-sw/cms-bot that referenced this pull request May 28, 2019
@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26967/10022

  • This PR adds an extra 36KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26967/10026

  • This PR adds an extra 36KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @maseguracern for master.

It involves the following packages:

L1Trigger/RPCTriggerPrimitives

The following packages do not have a category, yet:

L1Trigger/RPCTriggerPrimitives
Please create a PR for https://github.com/cms-sw/cms-bot/blob/master/categories_map.py to assign category

@cmsbuild can you please review it and eventually sign? Thanks.
@Martin-Grunewald this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@smuzaffar
Copy link
Contributor

assign l1

@cmsbuild
Copy link
Contributor

New categories assigned: l1

@rekovic you have been requested to review this Pull request/Issue and eventually sign? Thanks

@maseguracern
Copy link
Contributor Author

Yes, +1

@rekovic
Copy link
Contributor

rekovic commented May 30, 2019

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 30, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/605/console Started: 2019/05/30 15:23

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 4, 2019

Pull request #26967 was updated. @cmsbuild, @rekovic, @benkrikler can you please check and sign again.

@fabiocos
Copy link
Contributor

fabiocos commented Jul 4, 2019

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 4, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/1328/console Started: 2019/07/04 16:34

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 4, 2019

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 4, 2019

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 4, 2019

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-96a93f/1328/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 33
  • DQMHistoTests: Total histograms compared: 3160399
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3160064
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
  • Checked 137 log files, 14 edm output root files, 33 DQM output files

@fabiocos
Copy link
Contributor

fabiocos commented Jul 5, 2019

@rekovic I am satisfied, please check and in case sign it or comment further

@rekovic
Copy link
Contributor

rekovic commented Jul 9, 2019

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 9, 2019

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@fabiocos
Copy link
Contributor

fabiocos commented Jul 9, 2019

+1

@cmsbuild cmsbuild merged commit fdb7b71 into cms-sw:master Jul 9, 2019
deadSource = cms.string('File'),
deadvecfile = cms.FileInPath('RecoLocalMuon/RPCRecHit/data/RPCDeadVec.dat'),
recAlgoConfig = cms.PSet(),
recAlgo = cms.string('RPCRecHitStandardAlgo')
Copy link
Contributor

Choose a reason for hiding this comment

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

The plugin named RPCRecHitStandardAlgo is not in CMSSW and is causing large number of workflows in the integration builds to fail.

Copy link
Contributor

Choose a reason for hiding this comment

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

It actually does exist, but is associated to a different plugin factory.

#include "FWCore/PluginManager/interface/PluginFactory.h"
#include "L1Trigger/RPCTriggerPrimitives/plugins/PrimitiveAlgoFactory.h"

EDM_REGISTER_PLUGINFACTORY(PrimitiveAlgoFactory, "PrimitiveAlgoFactory");
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a big problem. The exact same Factory is already registered under a different name:

typedef edmplugin::PluginFactory<RPCRecHitBaseAlgo *(const edm::ParameterSet &)> RPCRecHitAlgoFactory;

One cannot registered two factories with identically signatures under two different names. Only one name will be used by the system which leads to problems loading the plugins.

This factory declaration should go away and the code should just use the existing RPCRecHitAlgoFactory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, I have problems by calling the rechitalgofactory, Does anyone know how can I solve?
L1TMuonRPCTriggerPrimitivesProducer.cc:(.text+0x28e): undefined reference to `edmplugin::PluginFactory<RPCRecHitBaseAlgo* (edm::ParameterSet const&)>::get()'

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have

<use name="RecoLocalMuon/RPCRecHit">

to your BuildFile.xml?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I do, but it does not work.

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem is the package RecoLocalMuon/RPCRecHit creates a plugin not a library and a plugin. Therefore you can't link to it and get the symbol you need. So it looks like files within RecoLocalMuon/RPCRecHit needs to be moved around so that at least RecoLocalMuon/RPCRecHit/src/RPCRecHitAlgoFactory.cc gets compiled into a library which is shared between the two packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

those changes generate similar errors.
tmp/slc7_amd64_gcc700/src/RecoLocalMuon/RPCRecHit/src/RecoLocalMuonRPCRecHit/RPCRecHitBaseAlgo.cc.o: In function RPCRecHitBaseAlgo::reconstruct(RPCRoll const&, RPCDetId const&, std::pair<__gnu_cxx::__normal_iterator<RPCDigi const*, std::vector<RPCDigi, std::allocator<RPCDigi> > >, __gnu_cxx::__normal_iterator<RPCDigi const*, std::vector<RPCDigi, std::allocator<RPCDigi> > > > const&, std::bitset<192ul> const&)': RPCRecHitBaseAlgo.cc:(.text+0x71): undefined reference to RPCClusterizer::doAction(std::pair<__gnu_cxx::__normal_iterator<RPCDigi const*, std::vector<RPCDigi, std::allocator > >, __gnu_cxx::__normal_iterator<RPCDigi const*, std::vector<RPCDigi, std::allocator > > > const&)'

Copy link
Contributor

Choose a reason for hiding this comment

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

Wasn't RPCRecHitStandardAlgo.cc one of the files to move out of src/ and into plugins/ ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, StandardAlgo is now in plugins

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you point us to your code?

@smuzaffar any ideas?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

/afs/cern.ch/user/m/masegura/CMSSW_10_6_0/src/RecoLocalMuon/RPCRecHit

@fabiocos
Copy link
Contributor

@Dr15Jones this is indeed a problem, thanks for your comments. Anyway @smuzaffar I wonder why this did not cause troubles neither in local PR tests nor in CMSSW_11_0_X_2019-07-09-2300, as it was merged earlier than yesterday evening IB

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