switch PHYSLITE configuration to block configuration

This switches to the new configuration mechanism. I had to add a number of options to the blocks to make this happen.

I haven't actually tested this yet, beyond ensuring that it runs in one configuration.

cc @boeriu @calpigia

I have two minor comments. Then as long as the output is exactly the same, I'm fine with the change.

I would put all this in a Cfg function maybe so it's more self-contained. Can still be in the same file.

added Derivation label

added 77 commits

• b28083ab...6e343547 - 76 commits from branch atlas:main
• c27858af - switch PHYSLITE configuration to block configuration

Compare with previous version

I implemented all the requested changes and will now mark it as ready to see if the PHYSLITE output changes.

Also adding @emmat who took over as derivation co-coordinator.

marked this merge request as ready

Jenkins please retry a build

This merge request affects 4 packages:

• PhysicsAnalysis/Algorithms/AnalysisAlgorithmsConfig
• PhysicsAnalysis/Algorithms/AsgAnalysisAlgorithms
• PhysicsAnalysis/Algorithms/EgammaAnalysisAlgorithms
• PhysicsAnalysis/DerivationFramework/DerivationFrameworkPhys

Affected files list will not be printed in this case

Adding @jcatmore ,@tadej ,@zmarshal ,@dshope ,@krumnack ,@emmat ,@calpigia as watchers

added Analysis CPAlgorithms Egamma analysis-review-required main review-pending-level-1 labels

Maybe to add this from the discussion on MatterMost: I had to disable the e-gamma SFs since they did not exist for the chosen working points for the Run 3 MC. This should be fine for PHYSLITE production, since that doesn't contain scale factors anyways, but maybe at some point we should change to working points that are actually defined/calibrated for Run 3 analysis.

CI Result SUCCESS (hash c27858af)

	Athena	AnalysisBase	AthAnalysis
externals
cmake
make
tests

Full details available on this CI monitor view. Check the JIRA CI status board for known problems
Athena: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
AthAnalysis: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST 79]

Did this and should this have run a test that we didn't change the output of PHYSLITE? I sort of assumed so, but if not, how do I run this manually?

You can do acmd.py diff-root <file1> <file2> (if you run in MP, you may need to add also --order-trees).

Ok, I had hoped for something far more automated, but I'll try to run one or two test jobs then.

It feels like something we should run in CI or at the very least in the nightly as at this point with PHYSLITE we could very easily pick up changes without noticing.

For some reason we don't even have PHYS fully frozen.

Maybe this should be revisited, tagging @emmat, @ekourlit, @gwatts for feedback.

I now did the test for one file, found and fixed one problem, and it seems to give me exact results, except for the photons which differ. This is likely because the old configuration didn't contain this fix for loose photons: !64056 (merged)

There is also a difference METAssoc_AnalysisMETAux, but I presume that is connected.

Can @emmat and @boeriu comment how to proceed? My suggestion would be to consider this another bugfix and merge it.

FYI, for the nightlies, months ago I set up a PHYS(LITE) monitoring system using DCube: https://daod-monitoring.web.cern.ch/index.html.

The distributions of all variables are checked against a reference file (currently it should be the 24.0.2 production). Some work is still needed (e.g. trimming the automatic histogram range, which in some cases is too wide, and maybe get some automatic alert system if some distribution is not "green"), but we can invest some time on it, if people will effectively benefit from it.

I'm not sure something similar can be done at CI level, since you will have to run on a much higher number of events (compared to the ones used for the current CI) to spot some changes in the distributions, no?

Regarding the MET, I would tend to agree to merge the bug fix. However, to be 100% sure, I would just double check with the experts that followed up on the MET differences in PHYSLITE recently, e.g. @schaarsc.

You can do the same test at the level of n-tuples instead of histograms. You already have the tests that produce PHYSLITE, so comparing to a reference n-tuple would not add much in terms of overall time needed for the test. It won't catch all possible changes (e.g. calibration changes only to 2 TeV electrons), but it will already catch a lot of changes and mistakes people makes.

If there is anything you want to check before merging please just let me know when you are done, if there is something I should do, and otherwise when you are finished.

From my side, it can go. Thanks!

Thanks for tagging me! I agree from my side as well, this one looks good to be merged.

Hi @krumnack,

@elmsheus pointed out that there are big changes in photon distributions since 2023-10-23T2101:

https://atlas-art-data.web.cern.ch/atlas-art-data/grid-output/main/Athena/x86_64-centos7-gcc11-opt/2023-10-24T2101/TrfTestsARTPlots/test_trf_q449_phys_physlite_ca/ecube/

Is it what you were expecting from this MR?

I'm not sure, this seems like a big difference, not just to photons, but also to electrons. And from what I can tell it is the low pt electrons/photons that disappear. We do not have a hard cut-off, which is a bit surprising, are the calibrations reapplied for those plots?

As I said, that may be the calibration files, but it is more likely to be the explicit pt cut: https://gitlab.cern.ch/atlas/athena/-/blob/main/PhysicsAnalysis/Algorithms/EgammaAnalysisAlgorithms/python/ElectronAnalysisConfig.py#L90-98 https://gitlab.cern.ch/atlas/athena/-/blob/main/PhysicsAnalysis/Algorithms/EgammaAnalysisAlgorithms/python/PhotonAnalysisConfig.py#L107-115

First question would be whether this is really the right cut. If so, we can just leave it as is.

Otherwise we have to decide what to do:

• we can just lower those cuts in those configs if that should be the default definition
• we can make the cut value configurable and then set it to a lower value or disable it for PHYSLITE
• there is some mechanic in the system that some cuts are not part of the preselection. this is something I copied from the sequence configuration and I'm not sure what exactly it is meant to do, but I could change the PHYSLITE thinning so that it only uses the preselection for the thinning

I'm not sure how best to proceed. Maybe some domain experts can chime in? @schaffer?

Hi there,

Just to say that there are no pt cuts in PhysLite, up to now. So do I understand correctly that the electron 4.5 GeV cut came in the the change to configurable?

Yes (change to configuration blocks). As I said, I can prepare a bugfix MR that makes that pt cut configurable and removes it in PHYSLITE, if we think that's the right way to do it.

Ok, done here (hope this just works, I wasn't able to test it due to the di-tau updates): !66717 (merged)

Thanks a bunch @krumnack!

Thanks. A more general point: I think that it is fine today to not apply any pt cuts. This may eventually be a way to reduce size, but I don't think we need to do this now.

Right now lets not accidentally add any cuts if we can to PHYSLITE.

Also, I'm a little surprised frozen PHYSLITE isn't in there.

We can add the frozen test (and also completely freeze PHYS if needed).

That isn't something for this MR, I suppose. What got me confused was I had thought that had gone in already. At any rate, take that part of the discussion offline from here.

added 1 commit

• 4d241bcd - switch PHYSLITE configuration to block configuration

Compare with previous version

This merge request affects 4 packages:

• PhysicsAnalysis/Algorithms/AnalysisAlgorithmsConfig
• PhysicsAnalysis/Algorithms/AsgAnalysisAlgorithms
• PhysicsAnalysis/Algorithms/EgammaAnalysisAlgorithms
• PhysicsAnalysis/DerivationFramework/DerivationFrameworkPhys

Affected files list will not be printed in this case

Adding @emmat ,@krumnack ,@jcatmore ,@tadej ,@zmarshal ,@calpigia ,@dshope as watchers

CI Result SUCCESS (hash 4d241bcd)

	Athena	AnalysisBase	AthAnalysis
externals
cmake
make
tests

Full details available on this CI monitor view. Check the JIRA CI status board for known problems
Athena: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
AthAnalysis: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST 81]

added 1 commit

• d85e1779 - switch PHYSLITE configuration to block configuration

Compare with previous version

This merge request affects 4 packages:

• PhysicsAnalysis/Algorithms/AnalysisAlgorithmsConfig
• PhysicsAnalysis/Algorithms/AsgAnalysisAlgorithms
• PhysicsAnalysis/Algorithms/EgammaAnalysisAlgorithms
• PhysicsAnalysis/DerivationFramework/DerivationFrameworkPhys

Affected files list will not be printed in this case

Adding @zmarshal ,@dshope ,@emmat ,@jcatmore ,@krumnack ,@tadej ,@calpigia as watchers

CI Result SUCCESS (hash d85e1779)

	Athena	AnalysisBase	AthAnalysis
externals
cmake
make
tests

Full details available on this CI monitor view. Check the JIRA CI status board for known problems
Athena: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
AthAnalysis: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST 82]