convertAll ignored after changes for IOSvc compatibility?

[giovannimarchiori]

Check duplicate issues.

• Checked for duplicates

Goal

I have a code that runs DDMarlinPandora on edm4hep files, via the edm4hep2LCIO converters:
https://gitlab.cern.ch/gmarchio/FCC-scripts/-/blob/main/run_ALLEGRO_reco.py?ref_type=heads#L972

Until today I was using for the input the legacy services (k4DataSvc, PodioInput) because the code would not run otherwise, but I was told yesterday that with
#216
key4hep/k4FWCore#280
I would be able to go back to the IOSvc input.

However, when I tried this today, I get multiple warnings in output that I didn't have before, one for each hit collection that I am trying to read in DDMarlinPandora, such as:

[ MESSAGE "DDMarlinPandora"] Failed to extract hcal calo hit collection: HCalBarrelReadoutPositioned, lcio::DataNotAvailableException: LCEventImpl::getCollection: collection not in event:HCalBarrelReadoutPositioned
These warnings only disappear if I modify the EDM4hepTool configuration of my code, which was:

    from Configurables import EDM4hep2LcioTool
    edm4hepConvTool = EDM4hep2LcioTool("EDM4hep2lcio")
    edm4hepConvTool.convertAll = True
    edm4hepConvTool.collNameMapping = {
        "MCParticles": "MCParticle",
        "TracksFromGenParticles": "TrackCollection",
    }
    pandora.EDM4hep2LcioTool = edm4hepConvTool

to include in the collNameMapping all the hits collections, for instance
"HCalBarrelReadoutPositioned": "HCalBarrelReadoutPositioned",

Before, this didn't seem to be necessary - it seemed to be handled automatically when convertAll = True as in my case. Is that a bug or a feature of the new changes?

Tagging @jmcarcell and @tmadlener

Cheers,
Giovanni

Operating System and Version

Rocky Linux 9

compiler

GCC 14.2.0 (Spack GCC)

The version of the key4hep stack

today's nightly

Package Version

main

Reproducer

source /cvmfs/sw-nightlies.hsf.org/key4hep/setup.sh

ddsim --enableGun --gun.distribution uniform --gun.momentumMin "10.000000*GeV" --gun.momentumMax "10.000000*GeV" --gun.thetaMin 1.553343 --gun.thetaMax 1.588250 --gun.phiMin 0.000000 --gun.phiMax 6.283185 --gun.particle mu- --numberOfEvents 1 --outputFile root/allegro_v03_evts_1_pdg_13_MomentumMinMax_10.0_10.0_GeV_ThetaMinMax_89.0_91.0_PhiMinMax_0_360_sim.root --random.enableEventSeed --random.seed 4242 --compactFile $K4GEO/FCCee/ALLEGRO/compact/ALLEGRO_o1_v03/ALLEGRO_o1_v03.xml

k4run run_ALLEGRO_reco.py --pandora --inputFiles "root/allegro_v03_evts_1_pdg_13_MomentumMinMax_10.0_10.0_GeV_ThetaMinMax_89.0_91.0_PhiMinMax_0_360_sim.root" --outputFile root/allegro_v03_evts_1_pdg_13_MomentumMinMax_10.0_10.0_GeV_ThetaMinMax_89.0_91.0_PhiMinMax_0_360_HCal_ON_digi_reco.root  --includeHCal --addTracks --calibrateClusters

Additional context

No response

[tmadlener]

Thanks for the report (and for testing this in the real world). This sounds like a bug to me, but I am wondering a bit why it didn't already show up in our tests, as we do in principle run a converter from EDM4hep to LCIO with convertAll = True there with and without IOSvc.

I will have a look.

[giovannimarchiori]

Hi @tmadlener
I was wondering if you were able to reproduce this issue or if you need more context?
Thanks, cheers!

[tmadlener]

Hi @giovannimarchiori,

apologies this slipped through last week on my end. Following your steps exactly, I am running into the following issue, where it looks like the CellPositionsSimpleCylinderPhiThetaSegTool is not (yet) part of the Key4hep stack. I am not sure from where it should come from.

Traceback (most recent call last):
  File "/cvmfs/sw-nightlies.hsf.org/key4hep/releases/2025-02-19/x86_64-ubuntu24.04-gcc13.3.0-opt/k4fwcore/1f00424e1297e486ed7064464eddab342a51e97b_develop-ssqa3n/bin/k4run", line 258, in <module>
    main()
  File "/cvmfs/sw-nightlies.hsf.org/key4hep/releases/2025-02-19/x86_64-ubuntu24.04-gcc13.3.0-opt/k4fwcore/1f00424e1297e486ed7064464eddab342a51e97b_develop-ssqa3n/bin/k4run", line 184, in main
    load_file(file)
  File "/cvmfs/sw-nightlies.hsf.org/key4hep/releases/2025-02-19/x86_64-ubuntu24.04-gcc13.3.0-opt/k4fwcore/1f00424e1297e486ed7064464eddab342a51e97b_develop-ssqa3n/python/k4FWCore/utils.py", line 85, in load_file
    exec(code, globals())
  File "<string>", line 483, in <module>
ImportError: cannot import name 'CellPositionsSimpleCylinderPhiThetaSegTool' from 'Gaudi.Configurables' (unknown location)

I should however be able to try and come up with a reproducer for the actual problem myself probably.

[tmadlener]

OK. I have adapted one of our tests slightly and toggled convertAll = True here

k4MarlinWrapper/test/gaudi_opts/test_global_converter_maps.py

Lines 74 to 80 in f17eec6

	inputConverter = EDM4hep2LcioTool("InputConverter")
	inputConverter.convertAll = False
	inputConverter.collNameMapping = {
	"MCParticles": "MCParticles",
	"PseudoRecoParticles": "PseudoRecoParticles",
	}
	inputConverter.OutputLevel = DEBUG

and here

k4MarlinWrapper/test/gaudi_opts/test_global_converter_maps.py

Lines 92 to 95 in f17eec6

	mcTruthConverter = Lcio2EDM4hepTool("TrivialMCTruthLinkerConverter")
	mcTruthConverter.convertAll = False
	mcTruthConverter.collNameMapping = {"TrivialMCRecoLinks": "TrivialMCRecoLinks"}
	mcTruthConverter.OutputLevel = DEBUG

and for me this works with the IOSvc and all collections are converted in both directions, including remapping names.

I am not entirely sure yet why it doesn't work in your case, maybe putting the OutputLevel of the converters to DEBUG could give us a bit more information on what they are actually trying to convert.

[giovannimarchiori]

ToolSvc.EDM4hep...   INFO Converting all collections from EDM4hep to LCIO
ToolSvc.EDM4hep...  DEBUG Converting collection DCHCollection (storing it as DCHCollection)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::SimTrackerHit from input DCHCollection
ToolSvc.EDM4hep...  DEBUG Converting collection ECalBarrelModuleThetaMerged (storing it as ECalBarrelModuleThetaMerged)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::SimCalorimeterHit from input ECalBarrelModuleThetaMerged
ToolSvc.EDM4hep...  DEBUG Converting collection ECalBarrelModuleThetaMergedContributions (storing it as ECalBarrelModuleThetaMergedContributions)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::CaloHitContribution from input ECalBarrelModuleThetaMergedContributions
ToolSvc.EDM4hep...  DEBUG CaloHitContribution collection cannot be converted standalone. SimCalorimeterHit collection to be converted in order to be able to attach to them
ToolSvc.EDM4hep...  DEBUG Converting collection ECalEndcapTurbine (storing it as ECalEndcapTurbine)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::SimCalorimeterHit from input ECalEndcapTurbine
ToolSvc.EDM4hep...  DEBUG Converting collection ECalEndcapTurbineContributions (storing it as ECalEndcapTurbineContributions)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::CaloHitContribution from input ECalEndcapTurbineContributions
ToolSvc.EDM4hep...  DEBUG CaloHitContribution collection cannot be converted standalone. SimCalorimeterHit collection to be converted in order to be able to attach to them
ToolSvc.EDM4hep...  DEBUG Converting collection EventHeader (storing it as )
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::EventHeader from input EventHeader
ToolSvc.EDM4hep...  DEBUG Converting collection HCalBarrelReadoutPhiRow (storing it as HCalBarrelReadoutPhiRow)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::SimCalorimeterHit from input HCalBarrelReadoutPhiRow
ToolSvc.EDM4hep...  DEBUG Converting collection HCalBarrelReadoutPhiRowContributions (storing it as HCalBarrelReadoutPhiRowContributions)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::CaloHitContribution from input HCalBarrelReadoutPhiRowContributions
ToolSvc.EDM4hep...  DEBUG CaloHitContribution collection cannot be converted standalone. SimCalorimeterHit collection to be converted in order to be able to attach to them
ToolSvc.EDM4hep...  DEBUG Converting collection HCalEndcapReadout (storing it as HCalEndcapReadout)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::SimCalorimeterHit from input HCalEndcapReadout
ToolSvc.EDM4hep...  DEBUG Converting collection HCalEndcapReadoutContributions (storing it as HCalEndcapReadoutContributions)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::CaloHitContribution from input HCalEndcapReadoutContributions
ToolSvc.EDM4hep...  DEBUG CaloHitContribution collection cannot be converted standalone. SimCalorimeterHit collection to be converted in order to be able to attach to them
ToolSvc.EDM4hep...  DEBUG Converting collection LumiCalCollection (storing it as LumiCalCollection)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::SimCalorimeterHit from input LumiCalCollection
ToolSvc.EDM4hep...  DEBUG Converting collection LumiCalCollectionContributions (storing it as LumiCalCollectionContributions)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::CaloHitContribution from input LumiCalCollectionContributions
ToolSvc.EDM4hep...  DEBUG CaloHitContribution collection cannot be converted standalone. SimCalorimeterHit collection to be converted in order to be able to attach to them
ToolSvc.EDM4hep...  DEBUG Converting collection MCParticles (storing it as MCParticle)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::MCParticle from input MCParticles
ToolSvc.EDM4hep...  DEBUG Converting collection MuonTaggerBarrelPhiTheta (storing it as MuonTaggerBarrelPhiTheta)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::SimCalorimeterHit from input MuonTaggerBarrelPhiTheta
ToolSvc.EDM4hep...  DEBUG Converting collection MuonTaggerBarrelPhiThetaContributions (storing it as MuonTaggerBarrelPhiThetaContributions)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::CaloHitContribution from input MuonTaggerBarrelPhiThetaContributions
ToolSvc.EDM4hep...  DEBUG CaloHitContribution collection cannot be converted standalone. SimCalorimeterHit collection to be converted in order to be able to attach to them
ToolSvc.EDM4hep...  DEBUG Converting collection MuonTaggerEndcapPhiTheta (storing it as MuonTaggerEndcapPhiTheta)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::SimTrackerHit from input MuonTaggerEndcapPhiTheta
ToolSvc.EDM4hep...  DEBUG Converting collection SiWrBCollection (storing it as SiWrBCollection)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::SimTrackerHit from input SiWrBCollection
ToolSvc.EDM4hep...  DEBUG Converting collection SiWrDCollection (storing it as SiWrDCollection)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::SimTrackerHit from input SiWrDCollection
ToolSvc.EDM4hep...  DEBUG Converting collection TracksFromGenParticles (storing it as TrackCollection)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::Track from input TracksFromGenParticles
ToolSvc.EDM4hep...  DEBUG TracksFromGenParticlesAssociation is a link collection, converting it later
ToolSvc.EDM4hep...  DEBUG Converting collection VertexBarrelCollection (storing it as VertexBarrelCollection)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::SimTrackerHit from input VertexBarrelCollection
ToolSvc.EDM4hep...  DEBUG Converting collection VertexEndcapCollection (storing it as VertexEndcapCollection)
ToolSvc.EDM4hep...  DEBUG Converting type edm4hep::SimTrackerHit from input VertexEndcapCollection
ToolSvc.EDM4hep...  DEBUG Event: 0 Run: 0
ToolSvc.EDM4hep...  DEBUG Creating GlobalconvertedObjectsMap for this event since it is not already in the EventStore
[ MESSAGE "DDMarlinPandora"] Failed to extract ecal calo hit collection: ECalBarrelModuleThetaMergedPositioned, lcio::DataNotAvailableException: LCEventImpl::getCollection: collection not in event:ECalBarrelModuleThetaMergedPositioned
[ MESSAGE "DDMarlinPandora"] Failed to extract hcal calo hit collection: HCalBarrelReadoutPhiRowPositioned, lcio::DataNotAvailableException: LCEventImpl::getCollection: collection not in event:HCalBarrelReadoutPhiRowPositioned
[ MESSAGE "DDMarlinPandora"] Failed to extract muon hit collection: MuonTaggerBarrelPhiThetaPositioned, lcio::DataNotAvailableException: LCEventImpl::getCollection: collection not in event:MuonTaggerBarrelPhiThetaPositioned

as you can see the converter is not converting my digitised CaloHit collections like ECalBarrelModuleThetaMergedPositioned

Note that these digitised hit collections are not in the input sim file, but are created during this reconstruction step - so maybe the "problem" is there (i.e. it's not a bug but a feature, the converter only converts what's in the input), and I have to declare to the converter explicitly all the collections that are being created in memory that I want to translate? Anyway, this problem did not arise when I was using the legacy IO ..

[tmadlener]

I can't seem to reproduce that either. The test case I used above, also has the PseudoRecoParticles collection created on the fly and that is being converted with convertAll = True, even if I remove the collNameMapping parameter entirely.

I have also checked the implementation again and the when used with the IOSvc the converter actually get's all the collections to convert from the TES:

k4MarlinWrapper/k4MarlinWrapper/src/components/StoreUtils.cpp

Line 39 in 6fb6133

std::vector<std::string> getAvailableCollectionsFromStore(const AlgTool* thisClass,

So in principle, if the collection has been read or produced, it should be converted. I am a bit puzzled since you wrote that this worked with the PodioInput and also that it works if you place the name into the collNameMapping for the converter. In the test case I use the algorithm that produces the PseudoRecoParticles is a functional algorithm, is the CreatePositionedECalBarrelCells also a Functional algorithm? That would be the only real difference at the moment that I can think of.

[tmadlener]

OK. I think I found the problem. When using the --pandora flag the workflow doesn't actually use the IOSvc (code in script). Instead it uses the old k4DataSvc and PodioInput. Can you try to see if things start working if you switch everything over to IOSvc?

If I try to run the same test using that, I can see that the test starts to fail for me as well. And it's the same reason as for you, the converter doesn't see collections that haven't been read from file.

I am not entirely sure how to fix it at the moment, as the interactions between these two ways of doing things have become quite complex in the meantime.

@jmcarcell could the approach of simply asking the TES for collections also work for the k4DataSvc approach?

[giovannimarchiori]

Hi @tmadlener
what I didn't say in the initial post is that the script you have been looking at was the version that I was using before the latest changes to enable IOSvc and the E4H<->LCIO converted work together were implemented in k4FWCore (sorry I didn't know you were going to try it out). The version that had the switch to IOSvc and where I observed the problems was in my local working area, I hadn't pushed it to the repo.
Now I just pushed to the repo: https://gitlab.cern.ch/gmarchio/FCC-scripts/-/blob/main/run_ALLEGRO_reco.py

I still observe the issue I initially reported.
I removed the muon digitiser that was causing you problems (because it relies on tools not yet in the nightly) so you should be able to reproduce the problem.

Recipe is still the same:

source /cvmfs/sw-nightlies.hsf.org/key4hep/setup.sh

ddsim --enableGun --gun.distribution uniform --gun.momentumMin "10.000000*GeV" --gun.momentumMax "10.000000*GeV" --gun.thetaMin 1.553343 --gun.thetaMax 1.588250 --gun.phiMin 0.000000 --gun.phiMax 6.283185 --gun.particle mu- --numberOfEvents 1 --outputFile root/allegro_v03_evts_1_pdg_13_MomentumMinMax_10.0_10.0_GeV_ThetaMinMax_89.0_91.0_PhiMinMax_0_360_sim.root --random.enableEventSeed --random.seed 4242 --compactFile $K4GEO/FCCee/ALLEGRO/compact/ALLEGRO_o1_v03/ALLEGRO_o1_v03.xml

k4run run_ALLEGRO_reco.py --pandora --inputFiles "root/allegro_v03_evts_1_pdg_13_MomentumMinMax_10.0_10.0_GeV_ThetaMinMax_89.0_91.0_PhiMinMax_0_360_sim.root" --outputFile root/allegro_v03_evts_1_pdg_13_MomentumMinMax_10.0_10.0_GeV_ThetaMinMax_89.0_91.0_PhiMinMax_0_360_HCal_ON_digi_reco.root  --includeHCal --addTracks --calibrateClusters

inside run_ALLEGRO_reco.py, in L17, there is a useIOSvc flag.

With IOSvc False, all collections are converted, including the "XXXXPositioned" collections that contained the digitised cells, as you can see from the debug messages of the converted.
With IOSvc True, the digitised cell collections created in memory (not existing in the input file) are not converted - unless one adds them to the CollNameMapping (code currently commented in L1043-1045.

[tmadlener]

I seem to still be missing something on my end, I get

[ MESSAGE "DDMarlinPandora"] DDPandoraPFANewProcessor - Init
[ ERROR "DDMarlinPandora"] Failed to initialize marlin pandora: std exception  getExtension: selection is not unique (or empty)  includeFlag: DetType( 0x10000) : COIL,  excludeFlag: DetType( 0x0) :  --- found detectors : 
DDMarlinPandora     FATAL Standard std::exception is caught in sysInitialize

[giovannimarchiori]

Hi @tmadlener I cleaned up the test a lot so that it should be easier for you to debug (and also to run the code successfully

just do in a fresh shell:

wget https://gmarchio.web.cern.ch/fcc/allegro/test.sh 
source test.sh

The script will download the steering file for k4run, the input sim file, setup today's nightly (where things are not broken) and run with and without IOSvc and compare the logs to show that with IOSvc the collections in the transient store not added explicitly to the mapping of the converter are not converted even if convertAll is True

Thanks!

[tmadlener]

Hi @giovannimarchiori, sorry for the delay on my side. Thanks a lot for very nice reproducer, worked like a charm :)

I have now understood why we don't see the failure in our tests but why it is clearly there for you. The CreatePositionedCaloCells is a Gaudi::Algorithm and not a Functional one. This means that it uses DataHandles to put things into the EventStore. The issue then is that these DataHandles talk to the k4DataSvc and not the IOSvc, and those two have small differences in the exact types they use to put things into the TES. Consequently the function that looks for all collections fails to cast this to the expected type here

k4MarlinWrapper/k4MarlinWrapper/src/components/StoreUtils.cpp

Lines 78 to 81 in 00ac363

	auto wrapper = dynamic_cast<AnyDataWrapper<std::unique_ptr<podio::CollectionBase>>*>(p);
	if (!wrapper) {
	continue;
	}

In the end that is why it is missing from "all collections".

@jmcarcell is this something that we can just fix easily here in the getAvailableCollectionsFromStore function? Or is this something that potentially runs deeper, since in principle we need to support Gaudi::Algorithm as well with the IOSvc.

[jmcarcell]

With the fix in #225, the logs are the same.