athena merge requestshttps://gitlab.cern.ch/atlas/athena/-/merge_requests2022-05-17T12:10:51+02:00https://gitlab.cern.ch/atlas/athena/-/merge_requests/50902[TrigConfiguration] Clean up destructor definitions2022-05-17T12:10:51+02:00Rafal Bielskirafal.bielski@cern.ch[TrigConfiguration] Clean up destructor definitionsThere are many changes in this MR but all are trivial and of the same kind. Adjust all classes in the TrigConfiguration domain to follow the destructor definition rules outlined below. I focused on this domain since it is core framework ...There are many changes in this MR but all are trivial and of the same kind. Adjust all classes in the TrigConfiguration domain to follow the destructor definition rules outlined below. I focused on this domain since it is core framework code (so should serve as an example of good code) and involves a lot of inheritance where this is relevant.
The rules followed are:
1. If the class is not involved in inheritance and is unlikely to be (e.g. defined as `final` or just a simple one use-case class without any `virtual` methods) and does not require a custom destructor, **do not explicitly declare the destructor**. This follows the "rule of zero". [1][2]
2. If the class is a base class for other classes (or may be and has `virtual` methods) and does not require a custom destructor, declare it as `virtual ~MyClass() = default;`. It is required to be defined to avoid undefined behaviour [3][4] and it is recommended to use `= default` for trivial destructors. [5][6]
3. If the class is derived, declare the destructor as `virtual ~MyClass() override;`. The `override` here is a clear additional indication of the inheritance and enforces that the base class has a virtual destructor defined, which prevents a class of bugs [7-10]. The `virtual` is redundant in this case [10][11] but arguably using both `virtual` and `override` may be useful and is actually recommended by ATLAS [12] as well as widespread and standard in the athena repository.
4. In addition to 3. if no custom implementation is required, declare the destructor as `virtual ~MyClass() override = default;`. This is the most common change in this MR.
----
On top of everything, adding the `override` to destructors uncovers other missing overrides in many classes since it triggers the `[-Winconsistent-missing-override]` compilation warning for classes where the `override` was incorrectly but consistently missing in multiple methods. Another good reason why this MR improves the code quality. The uncovered missing overrides were fixed in !51739.
----
[1] https://en.cppreference.com/w/cpp/language/rule_of_three
[2] https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rc-zero
[3] https://en.cppreference.com/w/cpp/language/destructor
[4] https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c35-a-base-class-destructor-should-be-either-public-and-virtual-or-protected-and-non-virtual
[5] https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#example-good-6
[6] https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-equals-default.html
[7] https://stackoverflow.com/questions/17923370/override-identifier-after-destructor-in-c11
[8] https://github.com/isocpp/CppCoreGuidelines/pull/1448
[9] https://github.com/isocpp/CppCoreGuidelines/issues/721
[10] https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-override.html
[11] https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rh-override
[12] [redeclare-virtual] in https://atlas-computing.web.cern.ch/atlas-computing/projects/qa/draft_guidelines.html#orgbb16391https://gitlab.cern.ch/atlas/athena/-/merge_requests/50901TrigHLTJetHypoUnitTests: Remove use of deprecated std::random_shuffle.2022-03-03T15:41:57+01:00Scott SnyderTrigHLTJetHypoUnitTests: Remove use of deprecated std::random_shuffle.Change uses of the deprecated algorithm std::random_shuffle to std::shuffle.Change uses of the deprecated algorithm std::random_shuffle to std::shuffle.https://gitlab.cern.ch/atlas/athena/-/merge_requests/50900StoreGate+PileUpTools: Remove references to deprecated functions.2022-03-03T15:42:51+01:00Scott SnyderStoreGate+PileUpTools: Remove references to deprecated functions.Remove (unused) references to deprecated std::bind1st, std::mem_fun,
std::not1. Clean up a couple other unused references.
Remove use of deprecated std::iterator.Remove (unused) references to deprecated std::bind1st, std::mem_fun,
std::not1. Clean up a couple other unused references.
Remove use of deprecated std::iterator.https://gitlab.cern.ch/atlas/athena/-/merge_requests/50899PyAnalysisUtils: python3 fix.2022-03-04T15:42:39+01:00Scott SnyderPyAnalysisUtils: python3 fix.string.lower no longer exists.string.lower no longer exists.https://gitlab.cern.ch/atlas/athena/-/merge_requests/50898MuonSegmentTaggerTools: Fix clang warning.2022-03-03T15:42:53+01:00Scott SnyderMuonSegmentTaggerTools: Fix clang warning.Unused variableUnused variablehttps://gitlab.cern.ch/atlas/athena/-/merge_requests/50896fix L1Calo define histograms options (ATLASRECTS-6856)2022-03-03T15:44:12+01:00Paul Thompsonfix L1Calo define histograms options (ATLASRECTS-6856)Fix mistakes with L1Calo overview define histogram - extra option for kLive was missing and online flag was incorrectly set. Causing problems in releaseFix mistakes with L1Calo overview define histogram - extra option for kLive was missing and online flag was incorrectly set. Causing problems in releasehttps://gitlab.cern.ch/atlas/athena/-/merge_requests/50895Fix older AthenaMP digitization ART jobs (ATLASSIM-5641)2022-03-03T15:44:05+01:00John Derek ChapmanFix older AthenaMP digitization ART jobs (ATLASSIM-5641)Some older Digitization ART jobs are using conditions tags which do not include the `/TRT/Calib/PID_NN` folder. This isn't needed in digitization itself, but a dependency seems to get picked up by the RDOMerging step. Most likely due to ...Some older Digitization ART jobs are using conditions tags which do not include the `/TRT/Calib/PID_NN` folder. This isn't needed in digitization itself, but a dependency seems to get picked up by the RDOMerging step. Most likely due to some lax configuration somewhere. This MR adds the required overrides to allow the RDO merging steps to run. These tests will be dropped soon anyway.
(As this MR only contains changes to ART scripts, the CI pipeline result will not be influenced in any way.)https://gitlab.cern.ch/atlas/athena/-/merge_requests/50894WorkflowTests: only use CVMFS for references2022-03-03T15:42:27+01:00Tadej Novaktadej.novak@cern.chWorkflowTests: only use CVMFS for referencesOnly use CVMFS for references in `WorkflowTests` for reliability reasons. Also this way tests behave the same also on machines without EOS.
For reference, CVMFS sync times are now more often at `{00,11,14,18}:18`.
Fizes ATLASRECTS-6839...Only use CVMFS for references in `WorkflowTests` for reliability reasons. Also this way tests behave the same also on machines without EOS.
For reference, CVMFS sync times are now more often at `{00,11,14,18}:18`.
Fizes ATLASRECTS-6839, related to ATLINFR-4435.
/cc @fwinkl @jchapman @ametehttps://gitlab.cern.ch/atlas/athena/-/merge_requests/50893CI: migrate Run 2 pile-up tests2022-03-04T09:18:58+01:00Tadej Novaktadej.novak@cern.chCI: migrate Run 2 pile-up testsMigrate Run 2 pile-up tests to the new infrastructure. As presampling and reco are now separate this required some minor tweaks to the test code.
Note I also reordered tests so that they go sim->digi->overlay.
/cc @fwinkl @jchapman
Re...Migrate Run 2 pile-up tests to the new infrastructure. As presampling and reco are now separate this required some minor tweaks to the test code.
Note I also reordered tests so that they go sim->digi->overlay.
/cc @fwinkl @jchapman
Relates to ATLINFR-4360.https://gitlab.cern.ch/atlas/athena/-/merge_requests/50891TrigMuonHypo: Demote verbose messages in TrigmuCombHypoTool from INFO to DEBUG2022-03-03T15:42:21+01:00Alaettin Serhan MeteTrigMuonHypo: Demote verbose messages in TrigmuCombHypoTool from INFO to DEBUGCloses ATR-24957Closes ATR-24957https://gitlab.cern.ch/atlas/athena/-/merge_requests/50890More robust check for chains in a given filter alg2022-03-04T21:43:01+01:00Teng Jian KhooMore robust check for chains in a given filter algThis is to fix a problem encountered with !50856 in which the the two following chains are in the menu:
* 'HLT_g40_loose_L1eEM24L'
* 'HLT_g40_loose_L1eEM24L_mu40_msonly_L1MU14FCH'
The latter requires the explicit 'L1eEM24L' on the first ...This is to fix a problem encountered with !50856 in which the the two following chains are in the menu:
* 'HLT_g40_loose_L1eEM24L'
* 'HLT_g40_loose_L1eEM24L_mu40_msonly_L1MU14FCH'
The latter requires the explicit 'L1eEM24L' on the first leg to disambiguate the copies seeded by legacy and Phase-I L1Calo RoIs.
In the JSON creation, the test for whether a chain is controlled by a given filter was performed by checking if the chain name was a substring of the filter's chains, because the filter could be checking specific leg IDs ('legXXX_[chainName]'). This leads to problems if one chain's name is a substring of another chain name, as is the case here. The solution is to compare the full names, stripping off the leg prefix if needed.https://gitlab.cern.ch/atlas/athena/-/merge_requests/50889default selection cuts for monitored offline tracks2022-03-04T21:43:24+01:00Sarka Todorovadefault selection cuts for monitored offline tracksSet default offline track selection cuts for monitoring to values which do not interfere with reconstruction cuts ( mb_sptrk )Set default offline track selection cuts for monitoring to values which do not interfere with reconstruction cuts ( mb_sptrk )Sarka TodorovaSarka Todorovahttps://gitlab.cern.ch/atlas/athena/-/merge_requests/50888Enable new CHS setting for EMPFlow jets (and simplifications)2022-03-21T14:21:50+01:00Pierre Antoine DelsartEnable new CHS setting for EMPFlow jets (and simplifications)We enable new setting for ChargedHadronSubtraction (CHS) procedure with pflow jets.
This new settings were approved by the JetEtmiss group.
At the same time, simplifications are made :
* TrackVertexAssociation are build from a dedicat...We enable new setting for ChargedHadronSubtraction (CHS) procedure with pflow jets.
This new settings were approved by the JetEtmiss group.
At the same time, simplifications are made :
* TrackVertexAssociation are build from a dedicated Alg (using the relevant CP tool) rather than a `JetAlgorithm` which uses `IJetExecuteTool` which uses a CP tool
* simplify other minor config functions
Tagging @haweber for confirmation of the CHS settings: the `JetTrackVtxAssoc` building is declared in `StandardJetConstits.py` and the CHS tool makes use of it at the end of the same file.
Also tagging @valentem : I changed a bit of the trigger config to make use of the simpler JetTrackVtxAssoAlg (0 impact on outputs expected)Pierre Antoine DelsartPierre Antoine Delsarthttps://gitlab.cern.ch/atlas/athena/-/merge_requests/50887Updating histo names for comparitor stage in el_zee_tier0_pu40_TnP nightly2022-03-03T15:42:11+01:00Harry SimpsonUpdating histo names for comparitor stage in el_zee_tier0_pu40_TnP nightlyComparitor stage was being terminated early due to a naming mismatch, this fixes that. JIRA: https://its.cern.ch/jira/browse/ATR-25017Comparitor stage was being terminated early due to a naming mismatch, this fixes that. JIRA: https://its.cern.ch/jira/browse/ATR-25017https://gitlab.cern.ch/atlas/athena/-/merge_requests/50886Migrating more jet features to new config2022-03-22T17:20:48+01:00Chris Malena DelitzschMigrating more jet features to new configThis MR removes the additional calls to decorate moments to the jets in the derivations after the jets have been built. Those moments are scheduled now automatically when building the jets. This is only done in the derivations though to ...This MR removes the additional calls to decorate moments to the jets in the derivations after the jets have been built. Those moments are scheduled now automatically when building the jets. This is only done in the derivations though to not schedule CPU intensive variables like fJVT also in the AODs.
Further changes include: applying calibration to large-R jets, protection against usage of towers when not present in file.
Tagging @delsart @schaarsc @sawyer @sjigginshttps://gitlab.cern.ch/atlas/athena/-/merge_requests/50885EvgenJobTransforms: add checkup of list with problems in specific processes2022-03-11T15:44:02+01:00Ewelina Maria LobodzinskaEvgenJobTransforms: add checkup of list with problems in specific processesEvgenJobTransforms: add check of a list with problems in specific processes
merged into 21.6 as !50865EvgenJobTransforms: add check of a list with problems in specific processes
merged into 21.6 as !50865https://gitlab.cern.ch/atlas/athena/-/merge_requests/50884TrigMinBiasMonitoring: Update to exclusive trigger monitoring2022-03-03T15:44:16+01:00Krzysztof Cieslakrzysztof.marcin.ciesla@cern.chTrigMinBiasMonitoring: Update to exclusive trigger monitoringThis MR is a follow-up to !50704. Adds requirement for a reference triggers to pass the same track selection as a monitored one.
Pinging @tboldThis MR is a follow-up to !50704. Adds requirement for a reference triggers to pass the same track selection as a monitored one.
Pinging @tboldhttps://gitlab.cern.ch/atlas/athena/-/merge_requests/50883CITest: migrate DQ tests2022-03-05T00:16:16+01:00Frank WinklmeierCITest: migrate DQ testsMigrate the DataQuality CI tests to the CITest package. During migration
the following changes were applied:
- change the input from Run-2 MC (q221) to Run-3 MC (q445)
- do not run reco itself but use the output of RecoRun3MC instead...Migrate the DataQuality CI tests to the CITest package. During migration
the following changes were applied:
- change the input from Run-2 MC (q221) to Run-3 MC (q445)
- do not run reco itself but use the output of RecoRun3MC instead
This also requires to (temporarily) change the output of the `RecoRun3MC` test from `AODSLIM` to `AODFULL` in order for the HLT monitoring to work (ATLASDQ-879).
Relates to ATLINFR-4360. cc @ponyisihttps://gitlab.cern.ch/atlas/athena/-/merge_requests/50882IOVSvc: add dropObjectFromDB method2022-03-03T15:43:23+01:00Frank WinklmeierIOVSvc: add dropObjectFromDB methodAdd a method to invalidate IOVs and drop the associated objects from the
IOVDbSvc. The latter could already be achieved by `IOVDbvSvc::dropObject`
but this new method takes the relevant lock before modifying a folder object.
Also adjust...Add a method to invalidate IOVs and drop the associated objects from the
IOVDbSvc. The latter could already be achieved by `IOVDbvSvc::dropObject`
but this new method takes the relevant lock before modifying a folder object.
Also adjust the unit test (`CondWriterExtAlg`) and the `TrigCOOLUpdateHelper` to use this new method.
Could potentially fix crashes seen in the HLT (ATR-24383).https://gitlab.cern.ch/atlas/athena/-/merge_requests/50881PLR_ID Class Implementation2022-03-15T21:10:30+01:00Tadej Novaktadej.novak@cern.chPLR_ID Class ImplementationIntroducing a `PLR_ID` class for the Pixel Luminosity Detectors to use instead of the `PixelID` class. This is a cleaned-up version of !48400 by @dfellers which does not introduce `PLR_ID` usage yet.
Having a separate ID class for the P...Introducing a `PLR_ID` class for the Pixel Luminosity Detectors to use instead of the `PixelID` class. This is a cleaned-up version of !48400 by @dfellers which does not introduce `PLR_ID` usage yet.
Having a separate ID class for the PLR is needed to avoid the issue described in this [JIRA ticket](https://its.cern.ch/jira/browse/ATLSWUPGR-145).
`PLR_ID` inherits from `PixelID` and uses the same external interfaces such that pixel tools/algorithms can use the `PLR_ID` functions just like it does the `PixelID` functions. A new `PLR_IDDetDescrCnv` class is used to create the `PLR_ID` class in the `DetectorStore`.
This ITkLayouts [MR](https://gitlab.cern.ch/Atlas-Inner-Tracking/ITKLayouts/-/merge_requests/202) needs to be merged before this will work, as the IdDictInnerDetector_ITK_LOCAL.xml dictionary needs to be updated in accordance with the `PLR_ID` scheme. This will redefine the InnerDetector Identifier part "TRT" to "LuminosityDetectors" and then have the next bit distinguish between PLR and BCM'.
Still running some offline tests before un-drafting...
/cc @spagan @pagessin @nstyles @schaffer