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

Pr90x l1t integration (v89.15) fix SumEt, uGT prescales, memory leaks, clean-up #16967

Merged

Conversation

rekovic
Copy link
Contributor

@rekovic rekovic commented Dec 11, 2016

This is PR 90x. Replaces #16718. (PR 80x version is #16966).

It is needed in 80x for multiple reasons:

  1. HI MC production
  2. Analysis of Trigger of 2016 runs

It is needed for 81x:

  1. If needed to run Stage2 calorimetry for L1T studies with Phase2 Tracker TDR samples.

Compared to #16717 it has:

  • fix to SumEt packer introduced in 8_0_22
  • fix in GT prescales to cover all 512 files
  • fixes a problem of a few missing commits

(For completion)PR #16717 Includes all fixes for the L1T emulator:

  • BMTF unpacker memory leak
  • TwinMux memory leak (already in 81x, so not present here)
  • Layer2 emulator to do what in FW (bug in FW tower max eta in TowerCount for HI)
  • Turn off warnings in L1T Ntuple maker.
  • uGT handling of saturated Et bins in CorrelationConditions to be as in FW.

eparadas and others added 29 commits November 22, 2016 03:07
…nasevich). L1CaloTowerTreeProducer produces spurious warning messages when trying to fill the CaloCluster tree. CaloClusters aren't produced in real data (only emulator).
Conflicts:
	L1Trigger/L1TGlobal/data
	L1Trigger/L1TMuon/data
…ust as the emulator does. This facilitates unit-test comparisions between simulated EtSums and SIM+PACK+UNPACK EtSums, as the order is preserved.
…t change in FW is to set MaxEta=27 for TowerCount. So we do the same here.
…->27 for TowerCount. Set as default. Revert caloStage2Params 3_3 to have as originally towerEtaMax 28 for TowerCount.
…ff needed by l1MuonReco.

Conflicts:
	L1Trigger/L1TMuon/data
Conflicts:
	L1Trigger/L1TMuon/data
@rekovic
Copy link
Contributor Author

rekovic commented Dec 11, 2016

+1

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

@rekovic
Copy link
Contributor Author

rekovic commented Dec 12, 2016

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 12, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/16962/console Started: 2016/12/12 03:51

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

@@ -36,7 +36,7 @@ namespace l1t
return (value == 0);
}

bool unpacking(const Block& block, UnpackerCollections *coll, std::map<int, qualityHits>& linkAndQual_, const bool& isNewFw)
bool unpacking(const Block& block, UnpackerCollections *coll, qualityHits& linkAndQual_, const bool& isNewFw)
Copy link
Contributor

Choose a reason for hiding this comment

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

@rekovic - in a followup PR, please change the name of linkAndQual_ to be comply with the usual naming convention (blah_ is for class member data)

@@ -36,7 +36,7 @@ namespace l1t
return (value == 0);
}

bool unpacking(const Block& block, UnpackerCollections *coll, std::map<int, qualityHits>& linkAndQual_, const bool& isNewFw)
bool unpacking(const Block& block, UnpackerCollections *coll, qualityHits& linkAndQual_, const bool& isNewFw)
Copy link
Contributor

Choose a reason for hiding this comment

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

otherwise, this function is really not used in CMSSW?

@@ -39,10 +39,10 @@
if stage2L1Trigger.isChosen():
if pA_2016.isChosen():
print "L1TCalorimeter Conditions configured for Stage-2 (2016 pA) trigger. "
from L1Trigger.L1TCalorimeter.caloStage2Params_2016_v3_3_HI_cfi import *
from L1Trigger.L1TCalorimeter.caloStage2Params_2016_v3_3_1_HI_cfi import *
Copy link
Contributor

Choose a reason for hiding this comment

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

in a separate PR, the 'isChosen()' will need to be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am sorry I missed it, what should be used instead of "isChosen()".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And is this replacement only needed for 90x PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

no, will be needed to back port too

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 4f05b52 into cms-sw:CMSSW_9_0_X Dec 12, 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

8 participants