Skip to content

Fix `hasFlag` and `_renamed_map` in AthConfigFlags

This week, we discovered some of the CaloRecGPU tests were not running.

After some investigation, I narrowed it down to the changes to AthConfigFlags in !69865 (merged).

The problem boils down to the fact that we use cloneAndReplace with the new argument to not disable the old flags (which may or may not be desirable in general, but makes sense in our use case and was working before this was merged). With the new definition for hasFlag, flags that indeed existed before the cloneAndReplace were being reported as non-existent, which may or may not have caused more problems in other configurations.

After additional investigation, I realized the code in _renamed_map was not appropriately taking into account the case where multiple renamed flags could map back to the same original flags. This is a problem that would have existed since the possibility to cloneAndReplace without disabling the previous keys was added in !68913 (merged). The solution would be to return not a direct map from old to new names, but a map from old names to a list of new names. (Or, in general, treat it as a multimap.)

Additionally, for _renamed_map to correctly handle this renaming with the old flags remaining valid, the less intrusive way seems to be changing cloneAndReplace to also set _renames of the old flags to themselves so that the old flags are also correctly populated in the (multi)map returned by _renamed_map.

The general behaviour of the code for all other cases should remain unchanged.

Merge request reports

Loading