PrForwardTrackingVeloFTZoneCache- Fails to load in dd4hep builds

added to epic &1

added dd4hep label

added RTA WP2 label

changed the description

I don't think the issue here is anything to do with the tracking algorithm itself. The issue happens on

      this->template addConditionDerivation<ZoneCache( const DeFT& )>( {DeFTDetectorLocation::Default},
                                                                       this->template inputLocation<ZoneCache>() );

an appears to be when dd4hep tries to crate the DeFT detector element.

@bcouturi Any ideas ?

I think the issue is the fact dd4hep is attempting to attach the condition to /world - This doesn't work, and is I now recall something I ran into with the RICH when I tried to do it. My workaround was to attach to either the RICH1 or RICH2 Detelems as appropriate.

If I change

   typename base_class_t::KeyValue{"FTZoneCache", "AlgorithmSpecific-" + name + "FTZoneCache"},

to

   typename base_class_t::KeyValue{"FTZoneCache", "/world/AfterMagnetRegion/T:AlgorithmSpecific-" + name + "FTZoneCache"},

Then I get past this error. I get another one, almost certainly the same thing, elsewhere.

@bcouturi Is attaching derived conditions to /world supposed to work, as this seems to be what happens by default if the user does not manually specify what to attach them to ?

Hi @jonrob, it it supposed to work indeed, and we have several conditions attached there already. It looks like the problem is that the separator is ':', so the condition key should be

/world:AlgorithmSpecific-PrForwardTrackingVeloFTZoneCache

and not: /world#AlgorithmSpecific-PrForwardTrackingVeloFTZoneCache:

The algorithms themselves don't add /world at all to the tag, regardless of the separator. Where is this being added then for them ?

in https://gitlab.cern.ch/lhcb/Detector/-/blob/master/Core/src/DetectorDataService.cpp#L49 which is use in the DD4hepSvc

It looks like the use case you mention is not using that function.

Indeed, it doesn't specify anything w.r.t. the detector to attach to, which I think is the intention.

It then I guess gets to

https://gitlab.cern.ch/lhcb/LHCb/-/blob/master/Det/LbDD4hep/src/DD4hepSvc.cpp#L157

which looks like it is suppose to correctly handle the lack of a ":" by attaching to /world. I then get a bit lost in working out how /world# gets used instead of /world: ??

So, DD4hepSvc::add is indeed calling LHCb::Detector::parseConditionIdentifier to parse the key. I extended the debug printout to show what this is giving, and I get

HCb::Det::LbDD4hep::DD4hepSvc        DEBUG Adding derivation for  AlgorithmSpecific-PrForwardTrackingVeloFTZoneCache (3521AF0600000000) | DetElem='/world' Key='AlgorithmSpecific-PrForwardTrackingVeloFTZoneCache'

so indeed at this level the logic is detecting there is no ":" and automatically attaching to /world. So whatever is going wrong its deeper in... @bcouturi any idea why/where this could be happening ?

Indeed, that all seems correct, I need to reproduce the problem to understand what happens afterwards.

The # is a red herring, the exception is thrown by DD4hep which at that point displays the key with a '#' as separator: https://github.com/AIDASoft/DD4hep/blob/c4e56a5eb7ac8b2406e2c3c22e438087beff268d/DDCore/src/Conditions.cpp#L201

we are trying to build Panoptes to be able to attach it with gdb

My suspicion is the conditions key finds its way into different places. Its listed as the input location for the functional algorithm in question, so its used not only when the addConditionDerivation is made (and here I think things are handled) but also by the functional framework itself when the inputs to the algorithm are accessed. This use case is not obvious how to 'fix' as effectively we need to change the key the functional algorithm uses for access, and at the level this is done the algorithm has no way to know its a derived condition that needs it...

So, its not at all obvious to me if the only way to address this is to explicitly add the detector to attach to when the key is first defined. Something like what I have in

jonrob/Rec@1ef6c7fe

which is where I started to collect adding them as work arounds.

@bcouturi You don't need to build panoptes and everything it depends on to reproduce, as I believe #345 (closed) is the same basic issue, and there you can reproduce with 'just' a Moore test.

Actually with debugged with @clemenci: the issue seems to be with the DeFT itself in: https://gitlab.cern.ch/lhcb/LHCb/-/blob/master/Det/FTDet/include/FTDet/DeFTDetector.h#L24

The string should be "/world/AfterMagnetRegion/T/FT:DetElement-Info-IOV"

#345 (closed) is different, in that case it is failing to find the UT as the UT is not loaded in that job as shown in the job configuration.

Thanks ! I'll try with that.

Not to sound like a broken record, but could we please try and improve the diagnostic messages for cases like this ? Just seems to me we are wasting time (we can ill afford) debugging issues, when if the error reporting was better we would be fixing things way faster...

Yes, this works. takes me to the next new issue...

LHCb!3617 (merged)

@jonrob I agree with you, which is why we introduced LHCb!3604 (merged) last week. That is however not enough as proved here...

Actually the error message was telling us what to do:

...  ConditionsDependencyHandler: Failed to resolve conditon:A2C1868E00000000 @../Pr/PrAlgorithms/src/PrForwardTracking.cpp:1810 (input 0)

which means

go to line 1810 of PrForwardTracking.cpp and check the input 0

We got side tracked by the part of the message saying that we failed to set up the condition derivation for the output condition AlgorithmSpecific-PrForwardTrackingVeloFTZoneCache, but the error stated clearly that the problem was on the input and not on the output.

You are right, I was sidetracked by the fact that the log from DD4hep stated Exception while creating dependent Condition /world#AlgorithmSpecific-PrForwardTrackingVeloFTZoneCache:, where the effect of having a # instead of : would yield a key that has a null condition name (hence the 00000000).

@clemenci The meaning of the term (input 0) in the message completely escaped me.... So on that basis it wasn't perhaps as clear as you thought it was.

What I was thinking of as a clear message would have been something that actually mentioned /world/AfterMagnetRegion/T/FT-DetElement-Info-IOV stating this was not a valid string to a known Detelem, or something to that effect..

The issue is that at that point (i.e. in the callback that evaluates the derived condition) we only have the key and not the readable string. As a first workaround, we added to the exception the line number of the failing addDerivedCondition and the parameter number.

@bcouturi Sure, I understand you are doing the best you can given the limitations dd4hep places on us. I'm not 'complaining', just stating what I think the aspirations should be in terms of the clarity of the error reporting. I like to think I know a bit now about how all this works, more than the average developer I would say, but even for me the message wasn't quite clear enough to prevent me wandering off down the wrong rabbit hole..

And indeed it sent me and @bcouturi on the wrong path too. From this debugging session I learned how to read the error message we added just days ago ... Next step is to improve the error message with more clear details, but that requires a new release of DD4hep.

@clemenci There's one thing I still don't understand, which is given what now seems to be the issue, why did the changes I made in

jonrob/Rec@1ef6c7fe

Seemingly also help. And they did, which is why I was firmly stuck in the rabbit hole I was as it looked like the changes was addressing the issue at hand. Why would modifying the derived condition string seeming help with the incorrect DeFT path ???

Honestly I have no idea, but I'm not sure your change was actually working as something like

#ifdef USE_DD4HEP
                  {"UTGeomCache", "/dd/Structure/LHCb/BeforeMagnetRegion/UT:AlgorithmSpecific-" + name + "UTGeomCache"},
#else
                  {"UTGeomCache", "AlgorithmSpecific-" + name + "UTGeomCache"},
#endif

should fail because path /dd/Structure/LHCb does not exist in DD4hep (it's called /world).

Could it be that the condition derivation was ignored because the detector of the target condition did not exist?

Sorry, the UT changes where a bad example and indeed that path is wrong.

The SciFi changes though are correct as they use the right path and I am sure helped.

closed

PrForwardTrackingVeloFTZoneCache- Fails to load in dd4hep builds

Designs

Child items ...

Activity