Lack of collection ownership in Python allows reusing collections from an existing frame

[jmcarcell]

Discovered with @fredrikshaw, in Python it is possible to take a collection from a frame and writing it to another (or even the same) frame. For example, this code:

from podio.root_io import Reader, Writer
import podio
import edm4hep

input_file = 'edm4hep.root'
output_file = 'test.root'

reader = Reader(input_file)
event = reader.get('events')[0]
new_frame = podio.Frame()

for name in event.getAvailableCollections():
    new_frame.put(event.get(name), name)

writer = Writer(output_file)
writer.write_frame(new_frame, 'events')

is allowed and with the file that is produced with https://github.com/key4hep/EDM4hep/blob/main/scripts/createEDM4hepFile.py, it will show several stack traces but won't stop, and produce a readable file with some collections. When writing to the original frame it seems to fail for every put. This is not allowed in C++, since the collections that one gets from a frame are const, although one could still do a const_cast to bypass this but at least this is explicit. Note also that in the example above, if one excludes the types for which put fails:

    if name in ['RecoMCParticleLinkCollection', 'RecDqdxCollection', 'TrackerHitSimTrackerHitLinkCollection', 'CaloHitSimCaloHitLinkCollection', 'ParticleIDCollection', 'VertexRecoParticleLinkCollection']:
        continue

it seems to work completely fine, which could mean this code works most of the time unless reusing one of the types above. When it fails, the error that appears is:

/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/15.1.1/../../../../include/c++/15.1.1/bits/stl_deque.h:1433: reference std::deque<edm4hep::ReconstructedParticleObj *>::operator[](size_type) [_Tp = edm4hep::ReconstructedParticleObj *, _Alloc = std::allocator<edm4hep::ReconstructedParticleObj *>]: Assertion '__n < this->size()' failed.
 *** Break *** abort

I don't have an answer for this one. If we could know if a collection belongs in a frame then we could check in put and print some warning or error.

[m-fila]

I don't have an answer for this one. If we could know if a collection belongs in a frame then we could check in put and print some warning or error.

It'd be nicer to have a more direct way for checking this, but collection ID used to be assigned only to collection owned by a frame so it could be checked as a workaround. I think it could be also valid if we go with key4hep/k4FWCore#311 and change that collection ID is assigned to collections managed by something not just frame

[tmadlener]

We already track quite a bit of state around in any case:

podio/python/templates/Collection.h.jinja2

Lines 203 to 205 in a3e722f

	bool m_isValid{false};
	mutable bool m_isPrepared{false};
	bool m_isSubsetColl{false};

We could just pack them into a bitfield and carry another flag that we could check in Frame::put. We would just need to find a way to limit the accessibility for this.

Alternatively we could use the python Frame wrapper to attach a boolean to the collection (potentially via some pythonization) when we get it and then check that again for every put?

[m-fila]

We could just pack them into a bitfield and carry another flag that we could check in Frame::put. We would just need to find a way to limit the accessibility for this.

Right, this should solve it. I guess we'd like to check it in the C++ source for consistency, but it's a bit of shame since technically in C++ we don't need the check as we have proper ownership semantics.

Alternatively we could use the python Frame wrapper to attach a boolean to the collection (potentially via some pythonization) when we get it and then check that again for every put?

That might be a problem as some time ago we wanted to have a pythonization (opt-in) #663 to "freeze" objects and disallow adding new members since there were a few bugs related to that