Attempt at reading metadata flags from input

[SagarA17]

Hi,

This is an attempt to configure reading of metadata from input files. It's heavily motivated/directly picked up from TopCPToolkit (https://gitlab.cern.ch/atlasphys-top/reco/TopCPToolkit). You might not find it very well structured which I can understand. Please advise on how might want me to change this.

Attempting to solve #1664

Tagging @mdhank @tofitsch.

Cheers,
Sagar

[mdhank]

Hi @SagarA17 ,

I'm still investigating why, but one thing I've noticed so far is that this duplicates several output lines. E.g. before it would output

[^[[1m2023-12-14 16:16:29,484^[[0m][^[[1;37mINFO^[[0m ] Welcome to xAODAnaHelpers. Don't forget to read the documentation: https://xaodanahelpers.readthedocs.io/en/latest/. (^[[1mlogging.py^[[0m:9)

and now it outputs

[^[[1m2023-12-14 16:26:05,390^[[0m][^[[1;37mINFO^[[0m ] Welcome to xAODAnaHelpers. Don't forget to read the documentation: https://xaodanahelpers.readthedocs.io/en/latest/. (^[[1mlogging.py^[[0m:9)
Py:xAH ^[[1;37mINFO^[[0m Welcome to xAODAnaHelpers. Don't forget to read the documentation: https://xaodanahelpers.readthedocs.io/en/latest/.

It would be good to fix this before merging.

Can I also ask which AB versions require
#-DATLAS_USE_CUSTOM_CPACK_INSTALL_SCRIPT=TRUE is needed for some AB releases.?

Best,
Michael

[mdhank]

It looks like the duplicate logs are related to the
from AthenaConfiguration.AllConfigFlags import initConfigFlags
from AthenaConfiguration.AutoConfigFlags import GetFileMD
lines.

[SagarA17]

Hi @mdhank,

Thanks for taking a look. Firstly, the custom cpack flag is required 24.2.2 and onwards. See https://twiki.cern.ch/twiki/bin/viewauth/AtlasProtected/AnalysisReleaseCPackBug2023April

About the duplicate opening lines, do you have any suggestions on how to fix this? We cannot change the configuration within the imports, of course. Maybe something we can customize within the xAH logger?

Thanks,
Sagar

[mdhank]

Hi @SagarA17 ,

Just a point of clarification, it looks like the custom cpack flag is only required in 24.2.2 and 24.2.3, and is no longer required for 24.2.4 onwards. It would be useful to change the comment to specify these versions.

On a broader note for the PR, could you clarify for me what functionality is added? I see where the flags are set, but not where they are used. Are they processed internally by Athena? If so, my worry would be what happens if you set an option using the current methods that disagreed with the flags.

Best,
Michael

[SagarA17]

Hi @mdhank,

I thought that I could stop using the flag after 24.2.3 as well, but it's not true. See the CI jobs here - https://github.com/UCATLAS/xAODAnaHelpers/actions/runs/7211351053/job/19646592348?pr=1665
I added back the flag in the last commit above and the CI starts to pass, as you can see in this commit - 65a36f4.

The flags are used downstream for configurations, i.e. what campaign is an MC sample from, or what data period is being run on, to then configure correct SF files, PRW, etc. See the implementation here - https://gitlab.cern.ch/atlas-phys/exot/ueh/EXOT-2017-19/dhnl-algorithm-r22/-/blob/Sagar-dev/data/config_main.py?ref_type=heads#L54

Instead of having to pass such arguments during run time (like you can see is implemented in the parser), these flags can be passed instead by xAH using the athena methods. You can also see that I have configured the manual run time flags to override the athena driven flags, so functionally nothing should change.

My main reason for doing this flag check is minimum user configuration effort. This is also a step in the direction of the proposed "central ntuple production" for which you can just create a g-tag without having any manual run time configurations (such as the MC campaign), which can be used to run over a whole array of input DAODs across campaigns and MC/data.

[tofitsch]

It looks like the duplicate logs are related to the from AthenaConfiguration.AllConfigFlags import initConfigFlags from AthenaConfiguration.AutoConfigFlags import GetFileMD lines.

Hi, some insights on the duplicate log messages:

Adding e.g the line

logger.propagate = False

to python/logger.py before the Welcome to xAODAnaHelpers... messages removes the duplicate of that message in the logs.

I don't suggest to do that. But that points to where the issue might be coming from.

I suspect that something in athena/Control/AthenaConfiguration loads a logger without name, making it the root logger to which all other loggers propagate their outputs.
As a result we get each logging message first from the dedicated xAH logger for it and then from the root logger imported with the newly added AthenaConfiguration imports in scripts/xAH_run.py

Looking further into this now.

[tofitsch]

So, when importing anything from AthenaConfiguration athena will set the root logger logging.root of the whole project to it's own version AthenaLogger:
https://gitlab.cern.ch/atlas/athena/-/blob/release/24.2.32/Control/AthenaCommon/python/Logging.py#L87

Changing

logging.root = AthenaLogger( 'root', logging.INFO )
logging.Logger.root = logging.root

into

logging.Logger.root = AthenaLogger( 'root', logging.INFO )

removes all the duplicates.
But we don't really want to change that in athena.

Instead, we should ensure to keep the original root logger. I propose to change the new import lines in xAH_run.py that cause this issue:

from AthenaConfiguration.AllConfigFlags import initConfigFlags
from AthenaConfiguration.AutoConfigFlags import GetFileMD

into:

# the following imports overwrite the root logger with Athenalogger
# we therefore save the original root logger here and afterwards set it to be root logger again
xAH_root_logger = logging.root

from AthenaConfiguration.AllConfigFlags import initConfigFlags
from AthenaConfiguration.AutoConfigFlags import GetFileMD

logging.Logger.root = xAH_root_logger
logging.Logger.manager = logging.Manager(xAH_root_logger)

That prevents all duplicates.

[SagarA17]

Hi @tofitsch,

Thanks for looking into it! I have implemented your suggestion to fix the double logging. I have also moved the ROOT import upstream, since the athena imports were intrinsically calling ROOT and it was opening an x11 window for me in an ssh session. Hence, I set a batch mode explicitly.

[mdhank]

Hi @SagarA17 ,

Thanks for the fixes, and thanks to @tofitsch for figuring this out!

You can also see that I have configured the manual run time flags to override the athena driven flags, so functionally nothing should change.

I can see this in your code, but there's no built-in protection on the xAODAnaHelpers side. So a user could get the flags and choose not to apply them. This is probably fine since the flags don't do anything unless the user uses them as input, but I would still recommend including a warning when flags are turned on that they will not be automatically applied by xAODAnaHelpers.

Longer-term, a more ambitious option could be to find everywhere xAODAnaHelpers uses these options and have it automatically use the flag values when provided (unless a contradictory option is provided manually). But this would be a much bigger project and I think outside the scope of this PR.

Not directly related to the PR but possibly helpful for your config file, some of these can already be detected automatically. For example, you can provide the PRW actualmu files for all 3 campaigns even if you're only running on one- they are only used in https://github.com/UCATLAS/xAODAnaHelpers/blob/main/Root/BasicEventSelection.cxx#L1243-1251, which will only actually apply them for the campaigns in use, which it determines directly from the RunNumber if you set m_autoconfigPRW to True. This will also then add in the proper lumicalc files (see https://github.com/UCATLAS/xAODAnaHelpers/blob/main/Root/BasicEventSelection.cxx#L1272-1297).

Unrelated to the PR, I noticed you're only using the lowest pT electron and muon triggers. If my understanding is correct, for electrons R21 scale factors were only supported for the logical OR of the lowest unprescaled triggers, so R22 may be similar once they are available (EGamma twiki).

Best,
Michael

[mdhank]

Hi @SagarA17 ,

I just wanted to follow-up on this- I'd prefer a warning is added that the flags are not automatically propagated, but once that's done I'm ready to merge.

Best,
Michael

[SagarA17]

Hi @mdhank,

Thanks for the follow up! I was occupied with other things, and I will add a warning today making the PR ready.

Thanks,
Sagar

[SagarA17]

Hi @mdhank,

Thanks for waiting! I have added a simple warning, and now the PR should be ready to merge.

Cheers,
Sagar