added separate hypo for pileup cluster removal for the Exotic Jets. One can add 0XXpileuprm0XX at the beginning of the chain to schedule that hypo. As an example "HLT_j20_059pileuprm072_j70_j50a_j0_DJMASS900j50dphi260x200deta_L1MJJ-500-NFF", where 0.72 is a low threshold and 0.59 is a high threshold for EMF of a given jet. For now, it doesn't change calratio and calratiovar algos. To be changed in the next MRs.
Merge request reports
Activity
requested review from @zhongyuk
This merge request affects 4 packages:
- Trigger/TrigHypothesis/TrigHLTJetHypo
- Trigger/TrigValidation/TrigAnalysisTest
- Trigger/TrigValidation/TrigP1Test
- Trigger/TriggerCommon/TriggerMenuMT
Affected files list will not be printed in this case
Adding @iriu ,@cantel ,@shanisch ,@salderwe ,@martindl ,@ayana ,@ademaria ,@carquin ,@vmartin ,@gipezzul ,@sutt ,@lidiaz ,@okumura ,@slai ,@ggonella ,@valentem ,@peter ,@miochoa as watchers
-
-
Athena externals 
cmake make tests 
Full details available on this CI monitor view. Check the JIRA CI status board for known problems
Resolved by Teng Jian KhooHi @tovsiann, just to understand, would the follow-up require renaming the existing calratio(var) chains, or would you just schedule the PU cluster removal alg automatically in those?
- Last reply by Teng Jian Khoo
added review-pending-expert label
Need an HLT expert's advice here.
Am attempting to run chains that use the new code via the trigger ART tests. To do so, I add a new chain to the MC_pp_run3_v1.py (copied from Dev_pp_run3_v1.py, so presant in both menus.
My question is: should adding the new chain to the correct menu in Trigger/TriggerCommon/TriggerMenuMT cause the chain to be run in the trigger ART tests, invoked via runTrigART.py -m?
Hi @peter
This procedure should work here - thanks
git fetch "https://:@gitlab.cern.ch:8443/tovsiann/athena.git" 'fix2' git checkout -b 'athena-fix2' FETCH_HEAD asetup Athena,24.0,r07 # Tatiana's commit is on top of a commit from the 7th ## Compile these packages: #Trigger/TrigHypothesis/TrigHLTJetHypo #Trigger/TrigValidation/TrigAnalysisTest #Trigger/TrigValidation/TrigP1Test #Trigger/TriggerCommon/TriggerMenuMT test_trig_mc_v1Dev_build.pyremoved review-pending-level-1 label
added review-user-action-required label
With the help of the trigger experts, I was able to make a test run with a pileuprm chain. From the debug output I see that:
- there are two independent parts to the chain. Independent means that the each part looks at the incoming jets as if the other part were not present. Thus a given jet can participate to the success of both parts (jet sharing).
- The first part requires that there be >= 3 jets, where the following conditions are met by different jets:
- a jet with pt > 20Gev 0<|eta| <3.2, and m_max = 0,72, m_min = 0.59 where these last variables are attributes of the PileupRemovalCondition.
- a second jet with pt > 70 Gev, 0<|eta| <3.2
- a third jet with pt > 50 Gev, 0<|eta| <4.9
If this is what the authors intended, it looks as if they have met their goal.
Some niggles:
- the m_max and m_min are printed out with labels as which is max and which is min. It would be clearer if such labels were added. Alternatively present the cut as a window cut with the min value lowest.
- Looking at PileupRemovalCondition::isSatisfied(), it appears that m_min and m_max are used only to calculate LRmax and LRmin, and that this is done several times (depending on the number of jets in the event, possibly many times) each event. Could these not be calculated once and for all in the constructor, and either replace or supplement the m_min, m_max attributes, with suitable adjustments to the toString() member function?
I will continue to look at the code, and let you know of anything else in the very near future.
Hi, "Looking at PileupRemovalCondition::isSatisfied(), it appears that m_min and m_max are used only to calculate LRmax and LRmin, and that this is done several times (depending on the number of jets in the event, possibly many times) each event. Could these not be calculated once and for all in the constructor, and either replace or supplement the m_min, m_max attributes, with suitable adjustments to the toString() member function?" My bed didn't use it thoughtfully. I will switch to LRmin and LRmax either in the initialization or to LogR cuts in the chain naming.
Hi Tatiana,
I haven't finished thinking about this, but in case you are making immediate code changes..
-
I think that m_min and m_max are in a restricted range of [-inf, inf]. You might argue that the code which obtains these values from the chain name guarantees that parts of the excluded range are done, does it exclude all such illegal parts. Even if it foes, it might be prudent to check these values in the constructor.
-
double LRmax=log10(1./(float(m_max)) - 1.); and similarly for LRmin. Does this not mean that LRmax =< LRmin if m_max >= m_min? In which case are the variable names not confusing? Especially with the tests like if LR > LRmax... although I understand that LRmax means "the LR limit associated with m_max", and similarly for LRmin. If you choose to keep these variable names, a comment might be helpful.
-
the value of the bool variable 'pass' is set twice, and differently. No action, except the return of the value depends on the value of pass, and so the first setting appears to be ineffective. This also means that LRmax is not used?
-
Copyright statement should say 2024, not 2023.
I am not in a position to judge the meaning of the code. It looks detailed!
I will continue looking at the other modified or new files
-
other trivia:
Copyright needs fixing in PileupRemovalCondition.cxx,h and TrigJetConditionConfig_pileup.h,cxx
Remove inappropriate author in PileupRemovalCondition
I see there are changes also to TrigJetCRHypoTool.cxx, with commented out code eg
- // return false;.
- Are the messages still relevant? Code could be probably be sanitized here
However, I think I was asked to check the pileup removal code, so I leave this up to you.
Otherwise, seems ok.
added 111 commits
- 6ca33481...1759e744 - 101 earlier commits
- dd833392 - address issues in PileupRemovalCondition
- 01cc4c1a - address issues in PileupRemovalCondition
- 30a8398b - address issues in PileupRemovalCondition
- 4135191d - address issues in PileupRemovalCondition
- 3e66308a - address issues in PileupRemovalCondition
- 2b13f566 - address issues in PileupRemovalCondition
- 5737435b - address issues in PileupRemovalCondition
- 0b2e5b39 - address issues in PileupRemovalCondition
- dbec3703 - address issues in PileupRemovalCondition
- 01e60a59 - address issues in PileupRemovalCondition
Toggle commit list-
This merge request affects 86 packages:
- Calorimeter/CaloRec
- Control/AthenaConfiguration
- DataQuality/DataQualityConfigurations
- DataQuality/GoodRunsListsUser
- DataQuality/dqm_algorithms
- Database/AthenaPOOL/OutputStreamAthenaPool
- Database/EventIndex/EventIndexProducer
- Database/IOVDbSvc
- DetectorDescription/IdDictDetDescrCnv
- Event/EventContainers
- Event/xAOD/xAODMetaDataCnv
- ForwardDetectors/ALFA/ALFA_BeamTransport
- ForwardDetectors/ZDC/ZdcAnalysis
- ForwardDetectors/ZDC/ZdcNtuple
- ForwardDetectors/ZDC/ZdcRec
- Generators/EvgenJobTransforms
- InnerDetector/InDetCalibAlgs/PixelCalibAlgs
- InnerDetector/InDetConfig
- InnerDetector/InDetDigitization/TRT_Digitization
- InnerDetector/InDetMonitoring/InDetAlignmentMonitoringRun3
- InnerDetector/InDetValidation/InDetPhysValMonitoring
- LArCalorimeter/LArAlignment/LArAlignmentAlgs
- LArCalorimeter/LArCabling
- LArCalorimeter/LArCafJobs
- LArCalorimeter/LArCalibTools
- LArCalorimeter/LArCalibUtils
- LArCalorimeter/LArExample/LArCalibProcessing
- LArCalorimeter/LArMonitoring
- LArCalorimeter/LArROD
- LArCalorimeter/LArRecConditions
- LArCalorimeter/LArRecUtils
- MuonSpectrometer/MuonConfig
- MuonSpectrometer/MuonReconstruction/MuonDataPrep/STgcClusterization
- MuonSpectrometer/MuonReconstruction/MuonRecEvent/MuonPrepRawData
- MuonSpectrometer/MuonValidation/MuonDQA/MuonRawDataMonitoring/StgcRawDataMonitoring
- MuonSpectrometer/MuonValidation/MuonDQA/MuonTrackMonitoring
- PhysicsAnalysis/DerivationFramework/DerivationFrameworkL1Calo
- PhysicsAnalysis/JetTagging/FlavorTagDiscriminants
- PhysicsAnalysis/JetTagging/JetTagAlgs/BTagging
- PhysicsAnalysis/JetTagging/JetTagTools
- Projects/AthAnalysis
- Projects/AthGeneration
- Projects/AthSimulation
- Projects/Athena
- Projects/DetCommon
- Projects/VP1Light
- Reconstruction/Jet/JetRecConfig
- Reconstruction/RecExample/RecExRecoTest
- Reconstruction/egamma/egammaConfig
- Reconstruction/tauRec
- Simulation/Digitization/DigitizationConfig
- Simulation/G4Atlas/G4AtlasAlg
- Simulation/G4Atlas/G4AtlasInterfaces
- Simulation/G4Atlas/G4AtlasTools
- Simulation/G4Utilities/G4FastSimulation
- Simulation/ISF/ISF_FastCaloSim/ISF_FastCaloSimParametrization
- Simulation/SimulationConfig
- Simulation/Tests/DigitizationTests
- Simulation/Tests/ISF_Validation
- TileCalorimeter/TileTBRec
- Tools/PROCTools
- Tools/PyJobTransforms
- Tools/Tier0ChainTests
- Tools/WorkflowTestRunner
- Trigger/TrigAccel/TrigGpuTest
- Trigger/TrigAlgorithms/TrigCaloRec
- Trigger/TrigAlgorithms/TrigTauRec
- Trigger/TrigConfiguration/TrigConfBase
- Trigger/TrigHypothesis/TrigEgammaHypo
- Trigger/TrigHypothesis/TrigHLTJetHypo
- Trigger/TrigHypothesis/TrigTauHypo
- Trigger/TrigMonitoring/TrigBjetMonitoring
- Trigger/TrigMonitoring/TrigMETMonitoring
- Trigger/TrigMonitoring/TrigTauMonitoring
- Trigger/TrigSteer/TrigOutputHandling
- Trigger/TrigT1/L1CaloFEX/L1CaloFEXByteStream
- Trigger/TrigT1/TrigT1CTP
- Trigger/TrigT1/TrigT1CaloMonitoring
- Trigger/TrigT1/TrigT1NSWSimTools
- Trigger/TrigValidation/TrigAnalysisTest
- Trigger/TrigValidation/TrigP1Test
- Trigger/TrigValidation/TrigValTools
- Trigger/TrigValidation/TriggerTest
- Trigger/TriggerCommon/TrigEDMConfig
- Trigger/TriggerCommon/TriggerJobOpts
- Trigger/TriggerCommon/TriggerMenuMT
Affected files list will not be printed in this case
Adding @sekula ,@iriu ,@lidiaz ,@adbailey ,@stsuno ,@jburr ,@bernius ,@dguest ,@mnowak ,@ggonella ,@peter ,@amelo ,@shanisch ,@pscholer ,@damazio ,@carquin ,@schaarsc ,@iouri ,@valentem ,@pavol ,@vmartin ,@sutt ,@jchapman ,@ssnyder ,@emmat ,@rbianchi ,@lshan ,@jbeirer ,@martindl ,@dbakshig ,@xiaozhon ,@thompson ,@svanstro ,@battagl ,@solodkov ,@fwinkl ,@bkerridg ,@keli ,@khamano ,@jodafons ,@kluit ,@akraszna ,@stelzer ,@cmorenom ,@cantel ,@jmharris ,@ayana ,@will ,@slai ,@oliveirg ,@gipezzul ,@lgagnon ,@jlieberm ,@okuprash ,@hrussell ,@apsallid ,@calfayan ,@cgrefe ,@jbeaucam ,@gemmeren ,@goetz ,@fpastore ,@lstocker ,@jajimene ,@rosati ,@miochoa ,@abarton ,@zhangr ,@ademaria ,@tamartin ,@mverissi ,@lbeemste ,@pagessin ,@bdong ,@martis ,@dossantn ,@jburzyns ,@amete ,@tadej ,@paulama ,@orlando ,@cvarni ,@suyogs ,@jcatmore ,@eegidiop ,@asonay ,@salderwe ,@sroe ,@tstreble ,@harkusha ,@krumnack ,@fernando ,@okumura ,@jojungge ,@bcarlson ,@aporeba ,@enagy ,@maszyman ,@stavrop ,@efurtado as watchers
added Analysis BTagging Build Calorimeter Core DQ Database Derivation Digitization EDM Egamma ForwardDetectors Generators Geometry ITk InnerDetector L1Calo LAr MuonSpectrometer Reconstruction Simulation Tau Tile Tools TriggerEDM TriggerMinBias analysis-review-required full-unit-tests review-pending-level-1 labels and removed review-pending-expert review-user-action-required labels
-
added 181 commits
-
01e60a59...896695e4 - 179 commits from branch
atlas:24.0 - 960eb523 - "separate pileup removal algo from calratio"
- 7809c095 - address issues in PileupRemovalCondition
-
01e60a59...896695e4 - 179 commits from branch
-
This merge request affects 4 packages:
- Trigger/TrigHypothesis/TrigHLTJetHypo
- Trigger/TrigValidation/TrigAnalysisTest
- Trigger/TrigValidation/TrigP1Test
- Trigger/TriggerCommon/TriggerMenuMT
Affected files list will not be printed in this case
Adding @shanisch ,@cantel ,@valentem ,@lidiaz ,@martindl ,@miochoa ,@gipezzul ,@peter ,@okumura ,@salderwe ,@ggonella ,@carquin ,@iriu ,@vmartin ,@ademaria ,@sutt ,@ayana ,@slai as watchers
-
removed Analysis label
removed BTagging label
removed Build label
removed Calorimeter label
removed Core label
removed Database label
removed Derivation label
removed Digitization label
removed EDM label
removed Egamma label
removed DQ label
removed ForwardDetectors label
removed Generators label
removed Geometry label
removed InnerDetector label
removed ITk label
removed JetEtmiss label
removed L1Calo label
removed LAr label
