[ntuple] fix auto schema evolution of base classes

[jblomer]

Forbid changing the number of base classes except for adding base classes where there were previously none and for removing all of the base classes.

[github-actions]

Test Results

    21 files      21 suites   3d 14h 57m 32s ⏱️
 3 691 tests  3 691 ✅ 0 💤 0 ❌
75 655 runs  75 655 ✅ 0 💤 0 ❌

Results for commit 22e3fe8.

♻️ This comment has been updated with latest results.

[krasznaa]

😦 Unfortunately this change is breaking our working model in ATLAS. 😦

Some of our transient classes look a little different in our full offline software versus our analysis software. For instance see:

https://gitlab.cern.ch/atlas/athena/-/blob/main/Event/xAOD/xAODCore/xAODCore/AuxInfoBase.h?ref_type=heads#L32

Here ILocakble is a type that we don't have in our analysis software. Since that type comes with no persistent information (in fact none of the xAOD base classes do), this was never an issue before. But now I see:

[bash][atspot01]:xaod > xAODRNFileReadTest /data/ATLAS/mc20_13TeV.OpenDataRNTuples.DAOD_PHYSLITE.p6026_p6898/DAOD_PHYSLITE.45469697._000003.pool.root.1
xAOD::Init                INFO    Environment initialised for data access
xAODFileReadTest          INFO    Opening file: /data/ATLAS/mc20_13TeV.OpenDataRNTuples.DAOD_PHYSLITE.p6026_p6898/DAOD_PHYSLITE.45469697._000003.pool.root.1
terminate called after throwing an instance of 'ROOT::RException'
  what():  incompatible number of base classes for field :_0: xAOD::AuxInfoBase, 3 base classes in memory  vs. 4 base classes on-disk
In-memory field/type hierarchy:
xTrigDecisionAux: [xAOD::TrigDecisionAuxInfo_v1, type version: 4294967295, type checksum: 2947148255] (id: 1)
  :_0 [xAOD::AuxInfoBase, type version: 4294967295, type checksum: 2833662725] (id: 2)
On-disk field/type hierarchy:
xTrigDecisionAux: [xAOD::TrigDecisionAuxInfo_v1, type version: 4294967295, type checksum: 3544918239] (id: 1)
  :_0 [xAOD::AuxInfoBase, type version: 4294967295, type checksum: 1617381893] (id: 2)

At:
  virtual std::unique_ptr<ROOT::RFieldBase> ROOT::RClassField::BeforeConnectPageSource(ROOT::Internal::RPageSource&) [/build1/atnight/localbuilds/nightlies/main--dev3LCG/build/build/AnalysisBaseExternals/src/ROOT/tree/ntuple/src/RFieldMeta.cxx:529]

 *** Break *** abort
    '/home/krasznaa/ATLAS/projects/xaod/build/x86_64-el9-gcc14-dbg/python',
    '/home/krasznaa/ATLAS/projects/xaod/build/x86_64-el9-gcc14-dbg/lib',
...

We will absolutely need help with this, as writing and then reading files with slightly different types is very deep in our analysis model. 🤔

[jblomer]

@krasznaa Should we move this to a separate issue?

It seems that a quick fix would be to define an empty dummy class in the analysis software, so that you can continue inheriting from it.

This would make the situation clear for the ROOT automatic schema evolution.

We removed the arbitrary removal and addition of base classes because, in general, it may not be clear what the schema evolution should do. Unlike with regular class members where we take the member name as a key, the type name of base classes is itself subject to change due to schema evolution (e.g., a base class T that evolves into U, where T and U are compatible types, such as vector<int> --> vector<long>). This can result in ambiguous cases or cases that don't compose (i.e., where you can go from class version N --> N+1 --> N+2 but not anymore from N --> N+2).

[krasznaa]

Hi Jakob,

Interesting idea. 🤔 Let me give it a try.

We already define separate versions of some classes for our "standalone" builds and the full ones. It would be fairly on brand with us to define a dummy version of ILockable for the analysis software as well. 🤔

As long as that doesn't end up causing other issues (I don't suppose it should...), that could be an acceptable solution for us.

[krasznaa]

Hi Jakob,

I believe we won't need to change this new behaviour after all. It seems to just force us into cleaning up our code a bit finally. See: https://gitlab.cern.ch/atlas/athena/-/merge_requests/83702