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.

Sign up for GitHub

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

Jump to bottom

Semi-random initial value in the L1 trigger prescale counter #37506

Merged
merged 7 commits into from May 25, 2022
Merged

Semi-random initial value in the L1 trigger prescale counter #37506

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
22b19aa
make L1 prescale start from a semi-random value
silviodonato Mar 28, 2022
ea3a94b
improve readibility, remove asserts
silviodonato May 1, 2022
17833d5
fix m_prescaleCounterAlgoTrig.reserve, move semirandomNumber outside …
silviodonato May 1, 2022
2c2d2e1
bugfix
silviodonato May 2, 2022
599560d
style fix
silviodonato May 2, 2022
1d2f9d3
move implementation of semirandom function to .cc file
silviodonato May 2, 2022
6d1821b
code checks
silviodonato May 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 39 additions & 1 deletion L1Trigger/L1TGlobal/interface/GlobalBoard.h
View file
Open in desktop
Expand Up @@ -14,6 +14,7 @@

// system include files
#include <bitset>
#include <cassert>
#include <vector>

// user include files
Expand All @@ -37,7 +38,7 @@

#include "FWCore/Framework/interface/Event.h"
#include "FWCore/Utilities/interface/InputTag.h"

#include "FWCore/MessageLogger/interface/MessageLogger.h"
#include "FWCore/Framework/interface/EventSetup.h"

// forward declarations
Expand Down Expand Up @@ -164,6 +165,39 @@ namespace l1t {
/// pointer to Tau data list
inline const BXVector<const GlobalExtBlk*>* getCandL1External() const { return m_candL1External; }

//initializer prescale counter using a semi-random value between [1, prescale value]
static const std::vector<double> semirandomNumber(const edm::Event& iEvent,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why static, rather than just a const class member?

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 wanted to stress that this is a standalone function, that might be used even outside a GlobalBoard object.
I might define it as an external function eg. l1t::semirandomNumber(...), but I don't know in which file I should define this function. I've just learned that I cannot define it as l1t::GlobalBoard::semirandomNumber(...) because a namespace cannot have the same name of a class.
I don't have a strong opinion. What do you suggest?

Copy link
Contributor

Choose a reason for hiding this comment

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

No strong opinion here, either. :)

I guess l1t::GlobalBoard::semirandomNumber(..) is effectively how this function would be called outside this class if using the current implementation. I don't see a good place for this elsewhere in this package, so unless L1T has a better idea, I would leave it here like this (maybe the implementation could be moved to the .cc file, though).

const std::vector<double>& prescaleFactorsAlgoTrig) {
auto out = prescaleFactorsAlgoTrig;
// pick a random number from a combination of run, lumi, event numbers
std::srand(iEvent.id().run());
std::srand(std::rand() + iEvent.id().luminosityBlock());
// this causes different (semi)random number number for different streams
// reminder: different streams have different initial event number
std::srand(std::rand() + iEvent.id().event());
// very large (semi)random number
double const semirandom = std::rand();
for (auto& ps : out) {
// if the ps is smaller than 1 (e.g. ps=0, ps=1), it is not changed
// else, replace ps with a semirandom integer in the [1,ps] range
if (ps > 1) {
auto nps = semirandom - floor(semirandom / ps) * ps;
// if nps=0 or a wrong value (<0,>ps) use PS value (standard method)
if (nps > 0 || nps <= ps)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (nps > 0 || nps <= ps)
if (nps > 0 and nps <= ps)

No?

ps = nps;
else {
if (nps != 0) // complain only if nps <0 or nps >PS
edm::LogWarning("L1TGlobal::semirandomNumber")
<< "\n The inital prescale counter obtained by L1TGlobal::semirandomNumber is wrong."
<< "\n This is probably do to the floating-point precision. Using the PS value." << semirandom
<< "\n semirandom = " << semirandom << "\n PS = " << ps << "\n nps = " << nps
<< " <-- it should be in the range [0 , " << ps << "]" << std::endl;
}
}
}
return out;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this never happens for the values used by this function, but for doubles in general I think the asserts above could fail (given floating-point precision and the fact that out[i] is the result of a computation). Is that why they are there?

The current impl only handles the case of non-fractional prescales, right?

Should the randomisation be per algo (meaning, a different rand for each algo), or not (like in the current impl)?

In case it helps, I find the impl below a bit more readable (but I didn't test it, and there are likely better ways than using std::round to check for integers).

      auto out = prescaleFactorsAlgoTrig;
      // pick a random number from a combination of run, lumi, event numbers (different number for different threads)
      std::srand(iEvent.id().run());
      std::srand(std::rand() + iEvent.id().luminosityBlock());
      // this causes different (semi)random number number for different streams
      // reminder: different streams have different initial event number
      std::srand(std::rand() + iEvent.id().event());
      // (semi)random number in [0,1] range
      auto const semirandom = std::rand() / double(RAND_MAX);
      for (auto& ps : out) {
        // if the ps is smaller than 1 (e.g. ps=0, ps=1) or if the prescale is fractional, it is not changed
        // else, replace ps with a semirandom integer in the [1,ps] range
        if (ps > 1 and ps == std::round(ps)) {
          ps = std::round((ps - 1)*semirandom + 1);
        }
      }
      return out;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like my reply is lost.

  • I replaced the assert with a warning. In case of problems it will simply use the standard method ( take PS )

  • I think that it should work also with fractional prescales, as they don't have a specific treatment in GlobalBoard.cc (not 100% sure though)

  • rand should be common to among all the algos in order to preserve the synchronization of the L1 prescales (eg. L1_ZeroBias1 with PS = 20 should be always in the shadow of L1_ZeroBias2 with PS =10 )

  • thanks, I partially copied your code. I had to make some change to preserve the behavior described above.

/* Drop individual EtSums for Now
/// pointer to ETM data list
inline const l1t::EtSum* getCandL1ETM() const
Expand Down Expand Up @@ -194,6 +228,7 @@ namespace l1t {
void setBxLast(int bx);

void setResetPSCountersEachLumiSec(bool val) { m_resetPSCountersEachLumiSec = val; }
void setSemiRandomInitialPSCounters(bool val) { m_semiRandomInitialPSCounters = val; }

public:
inline void setVerbosity(const int verbosity) { m_verbosity = verbosity; }
Expand Down Expand Up @@ -268,6 +303,9 @@ namespace l1t {

//whether we reset the prescales each lumi or not
bool m_resetPSCountersEachLumiSec = true;

// start the PS counter from a random value between [1,PS] instead of PS
bool m_semiRandomInitialPSCounters = false;
};

} // namespace l1t
Expand Down
3 changes: 3 additions & 0 deletions L1Trigger/L1TGlobal/plugins/L1TGlobalProducer.cc
View file
Open in desktop
Expand Up @@ -69,6 +69,7 @@ void L1TGlobalProducer::fillDescriptions(edm::ConfigurationDescriptions& descrip
// switch for muon showers in Run-3
desc.add<bool>("useMuonShowers", false);
desc.add<bool>("resetPSCountersEachLumiSec", true);
desc.add<bool>("semiRandomInitialPSCounters", false);
// These parameters have well defined default values and are not currently
// part of the L1T/HLT interface. They can be cleaned up or updated at will:
desc.add<bool>("ProduceL1GtDaqRecord", true);
Expand Down Expand Up @@ -116,6 +117,7 @@ L1TGlobalProducer::L1TGlobalProducer(const edm::ParameterSet& parSet)
m_requireMenuToMatchAlgoBlkInput(parSet.getParameter<bool>("RequireMenuToMatchAlgoBlkInput")),
m_algoblkInputTag(parSet.getParameter<edm::InputTag>("AlgoBlkInputTag")),
m_resetPSCountersEachLumiSec(parSet.getParameter<bool>("resetPSCountersEachLumiSec")),
m_semiRandomInitialPSCounters(parSet.getParameter<bool>("semiRandomInitialPSCounters")),
m_useMuonShowers(parSet.getParameter<bool>("useMuonShowers")) {
m_egInputToken = consumes<BXVector<EGamma>>(m_egInputTag);
m_tauInputToken = consumes<BXVector<Tau>>(m_tauInputTag);
Expand Down Expand Up @@ -197,6 +199,7 @@ L1TGlobalProducer::L1TGlobalProducer(const edm::ParameterSet& parSet)
m_uGtBrd = std::make_unique<GlobalBoard>();
m_uGtBrd->setVerbosity(m_verbosity);
m_uGtBrd->setResetPSCountersEachLumiSec(m_resetPSCountersEachLumiSec);
m_uGtBrd->setSemiRandomInitialPSCounters(m_semiRandomInitialPSCounters);

// initialize cached IDs

Expand Down
2 changes: 2 additions & 0 deletions L1Trigger/L1TGlobal/plugins/L1TGlobalProducer.h
View file
Open in desktop
Expand Up @@ -188,6 +188,8 @@ class L1TGlobalProducer : public edm::stream::EDProducer<> {

//disables reseting the prescale counters each lumisection (needed for offline)
bool m_resetPSCountersEachLumiSec;
// start the PS counter from a random value between [1,PS] instead of PS
bool m_semiRandomInitialPSCounters;
// switch to load muon showers in the global board
bool m_useMuonShowers;
};
Expand Down
13 changes: 10 additions & 3 deletions L1Trigger/L1TGlobal/src/GlobalBoard.cc
View file
Open in desktop
Expand Up @@ -971,10 +971,13 @@ void l1t::GlobalBoard::runFDL(edm::Event& iEvent,
// prescale counters are reset at the beginning of the luminosity segment
if (m_firstEv) {
// prescale counters: numberPhysTriggers counters per bunch cross
m_prescaleCounterAlgoTrig.reserve(numberPhysTriggers * totalBxInEvent);
m_prescaleCounterAlgoTrig.reserve(totalBxInEvent);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cms-sw/l1-l2 please note this is the only line which should affect the default behavior (semiRandomInitialPSCounters = false)


auto const& prescaleCountersAlgoTrig =
m_semiRandomInitialPSCounters ? semirandomNumber(iEvent, prescaleFactorsAlgoTrig) : prescaleFactorsAlgoTrig;

for (int iBxInEvent = 0; iBxInEvent <= totalBxInEvent; ++iBxInEvent) {
m_prescaleCounterAlgoTrig.push_back(prescaleFactorsAlgoTrig);
m_prescaleCounterAlgoTrig.push_back(prescaleCountersAlgoTrig);
}
m_firstEv = false;
m_currentLumi = iEvent.luminosityBlock();
Expand All @@ -984,7 +987,11 @@ void l1t::GlobalBoard::runFDL(edm::Event& iEvent,
if (m_firstEvLumiSegment || (m_currentLumi != iEvent.luminosityBlock() && m_resetPSCountersEachLumiSec)) {
m_prescaleCounterAlgoTrig.clear();
for (int iBxInEvent = 0; iBxInEvent <= totalBxInEvent; ++iBxInEvent) {
m_prescaleCounterAlgoTrig.push_back(prescaleFactorsAlgoTrig);
if (m_semiRandomInitialPSCounters) {
m_prescaleCounterAlgoTrig.push_back(semirandomNumber(iEvent, prescaleFactorsAlgoTrig));
} else {
m_prescaleCounterAlgoTrig.push_back(prescaleFactorsAlgoTrig);
}
}
m_firstEvLumiSegment = false;
m_currentLumi = iEvent.luminosityBlock();
Expand Down