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

Homework from PR 13768 #220

Open
mulhearn opened this issue Mar 31, 2016 · 2 comments
Open

Homework from PR 13768 #220

mulhearn opened this issue Mar 31, 2016 · 2 comments

Comments

@mulhearn
Copy link
Member

mulhearn commented Mar 31, 2016

Some issues were raised during PR cms-sw#13768 which we prefer to handle outside of that PR. We are tacking these here:

  • use of std::array instead of std::vector for ECAL lut for memory performance - this runs deep and is too hard to change at this moment
  • avoid using UCTRegion * as discussed in issue - agreed already this could wait
  • "GetGlobalEta: can we use M_PI?" - will do, but prefer not to break bitwise agreement with l1t-tsg-v4 and instead put in our dev branch.

Additional issues during PR cms-sw#14104

  • In L1Trigger/L1TCommon/plugins/L1TComparison.cc:
> @@ -361,6 +361,23 @@ L1TComparison::endJob() {
>    if (jetCheck_)  cout << "jet:   " << jetFails_ << "\n";
>    if (sumCheck_)  cout << "sum:   " << sumFails_ << "\n";
>    if (muonCheck_) cout << "muon:  " << muonFails_ << "\n";
> +
> +  int fail = 0;
@mulhearn - why not a bool?
  • In L1Trigger/L1TMuon/interface/L1TMuonGlobalParamsHelper.h:
> +  // double parameters indices
> +  enum dpIdx {maxdr=0, maxdrEtaFine=1};
> +
> +  // input enable indices
> +  enum linkNr {CALOLINK1=8, EMTFPLINK1=36, OMTFPLINK1=42, BMTFLINK1=48, OMTFNLINK1=60, EMTFNLINK1=66}; // link numbers start at 0
> +
> +  L1TMuonGlobalParamsHelper() { pnodes_.resize(NUM_GMTPARAMNODES); }
> +  L1TMuonGlobalParamsHelper(const L1TMuonGlobalParams &);
> +  ~L1TMuonGlobalParamsHelper() {}
> +
> +  // FW version
> +  unsigned fwVersion() const { return pnodes_[FWVERSION].uparams_.size() > FWVERSION_IDX ? pnodes_[FWVERSION].uparams_[FWVERSION_IDX] : 0; }
> +  void setFwVersion(unsigned fwVersion);
> +
> +  // Input disables
> +  std::bitset<72> inputsToDisable() const { return inputFlags(INPUTS_TO_DISABLE); };
@mulhearn - lets think about how to replace all of these magic numbers
@nsmith-
Copy link

nsmith- commented Apr 5, 2016

In addition, updates to the l1t-integration-CMSSW_8_0_2 branch since l1-tsg-v4 will cause a bit of a nasty merge conflict.
As soon as we get a new CMSSW release with this merge incorporated, I will create a 3-way patch to resolve the merge conflicts due to Layer 1 packages for you.

@nsmith-
Copy link

nsmith- commented Apr 12, 2016

This patch can be applied after merging l1t-integration-v34 into new CMSSW80X release that contains l1-tsg-v4 to resolve differences in L1Trigger/L1TCaloLayer1 package
https://ncsmith.web.cern.ch/ncsmith/L1TStage2Misc/merge_l1t-integration-v34..CMSSW80X.patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants