PrStoreUTHit - Problems loading dd4hep condition

Look at test

https://lhcb-nightlies.web.cern.ch/nightly/lhcb-dd4hep/1145/Moore/x86_64_v2-centos7-gcc11%2Bdd4hep-opt/tests#RecoConf.hlt2_light_reco_tracking

It fails with messages lkike

ConditionsDependencyHandler           ERROR Failed to resolve conditon:9C5E629397281372
ConditionDependency                   ERROR +++ Exception while creating dependent Condition 124D33D0 18BF8915:
ConditionDependency                   ERROR 		ConditionsDependencyHandler: Failed to resolve conditon:9C5E629397281372

which are not exactly helpful in determining where the actual issue is.

@bcouturi @sponce @clemenci Can you take a look please ?

Update - Following improvements to debugging info we now have

ConditionsDependencyHandler           ERROR Failed to resolve conditon:9C5E629397281372
ConditionDependency                   ERROR +++ Exception while creating dependent Condition 124D33D0 18BF8915:
ConditionDependency                   ERROR 		ConditionsDependencyHandler: Failed to resolve conditon:9C5E629397281372 @../Pr/PrAlgorithms/src/PrStoreUTHit.cpp:89 (input 0)

added to epic &1

added dd4hep label

changed the description

ping - Any ideas ? This issue is now affecting a fair number of Moore tests in the lhcb-dd4hep builds. I've no clue how to investigate, and the dd4hep messages are pretty useless in helping.

I'm trying to reproduce the problem with my development stack to debug the problem.

Thanks.

As much a solving the specific problem here, we really need to improve the diagnostics we get in cases like this. Assuming this sort of thing is going to crop up again, and I am sure it will (based on the principle whatever can go wrong will at some point) we need the messages to give some context on where the issue is, what sub-system, condition name etc. without the user having to delve deep into dd4hep to just figure that out. For me improving this is as important as fixing the specific issue here.

Indeed, you are right. This would however be best done in the DD4hep code itself. Now regarding this issue, I have problems reproducing the same problem in my stack (I get a different one with the "master" of all the projects).

With the _dbg platform I get:

ConditionDependency                   ERROR +++ Exception while creating dependent Condition /world#AlgorithmSpecific-PrStorePrUTHits-UTGeomCache:
ConditionDependency                   ERROR 		ConditionsDependencyHandler: Failed to resolve conditon:9C5E629397281372

so I expect this is trying to load the UT, whereas the DetectorList does not include the UT (as it's not ready):

# |-DetectorList          = ['/world', 'VP', 'FT', 'Magnet', 'Rich1', 'Rich2']
# |                       (default: ['/world', 'VP', 'FT', 'Magnet', 'Rich1', 'Rich2'])

I would go further : DD4hep obfuscation architecture is done well enough that we cannot do it in our code at all... Having said that, @bcouturi did manage to have some code merged in DD4hep around this, and you should be able to have more in debug mode.

@bcouturi what branches are you using. You probably need to use everything currently used in the lhcb-dd4hep slot builds to get to the point you see this issue, as otherwise you will hit other issues first.

We still don't have a way to show the name instead of the code when the condition is actually missing... And we need it in opt too...

Getting the right MRs into my stack takes time. In the end I used lbn-install to download the stack and reproduce the problem.

@sponce Indeed, the messages in the dbg builds are indeed a little more enlightening

https://lhcb-nightlies.web.cern.ch/nightly/lhcb-dd4hep/1146/Moore/x86_64_v2-centos7-gcc11%2Bdd4hep-dbg/tests

ConditionDependency                   ERROR +++ Exception while creating dependent Condition /world#AlgorithmSpecific-PrStorePrUTHits-UTGeomCache:
ConditionDependency                   ERROR 		ConditionsDependencyHandler: Failed to resolve conditon:9C5E629397281372

In any case, how can we disable PrStoreUTHits while the UT isn't ready/merged ?

See #336 (closed)

b.t.w. why are we only allowed to see the more useful message above in the debug builds ? I am not sure I see why we cannot have it in the opt builds as well, which is where we will normally first hit issues ?

Basically because the knowledge is completely lost in opt builds. There is nowhere in DD4hep opt mode the strings corresponding to hashes. DD4hep developers wanted to "optimize" memory usage and CPU. Not sure they've really managed... Anyway DD4hep should be rewritten from A to Z if you ask me.

To limit memory use, DD4hep conditions only have the string of the name attached to them in debug builds, otherwise only the hexadecimal key value is accessible...

I see. totally agree on the need to rewrite dd4hep. The whole design of the opaque cache approach to saving the data without any type info is something we need to push to get changed as it is frankly an absurd design choice that is causing a lot of headaches.

mentioned in issue LHCb#217 (closed)

I have spent some time trying how to understand this issue and for derived conditions the problem is deep in the DD4hep stack itself:

https://github.com/AIDASoft/DD4hep/blob/230efdcb2cf280f3351b8590431917244371185a/DDCond/src/ConditionsDependencyHandler.cpp#L299

The dependency is setup when you add the condition in: https://gitlab.cern.ch/lhcb/LHCb/-/blob/master/Det/LbDD4hep/src/DD4hepSvc.cpp#L166

The exception is only thrown once the callback is actually invoked. I suppose we could keep a map of the know keys in LHCb but at that point, I do not know how to even catch and interpret this from the DD4hepSvc: DD4hep throws a std::exception. If instead if threw a specialized exception from which we can get the missing key, we could translate it back from a readable string...

Missing exception when you directly invoke the DD4hepSvc are more easily dealt with as in that case you can you catch exceptions in the ConditionAccessorHolder and print the actual name before rethrowing...

Hi @jonrob,

As a workaround until we can fix DD4hep, I think we can do something like alongside the lines of: LHCb!3604 (merged)

For errors when the DD4hepSvc cannot find a specific condition: try/catch around the get call to enrich the exception with the detector name and path.

For errors due to missing dependencies to derived conditions, we have to implement the improvements in DD4hep which will take a while. What we can do is to dump in the log, in DEBUG mode the condition dependencies with full names and keys, as in:

LHCb::Det::LbDD...  DEBUG Adding derivation for  /world:DerivedCondition1 (124D33D097CF90D0)
LHCb::Det::LbDD...  DEBUG /world:DerivedCondition1 -> /world:TOTO (124D33D0A3526E3B)
LHCb::Det::LbDD...  DEBUG Adding derivation for  /world:DerivedCondition2 (124D33D0A99E346D)
LHCb::Det::LbDD...  DEBUG /world:DerivedCondition2 -> /world:TOTO (124D33D0A3526E3B)
LHCb::Det::LbDD...  DEBUG /world:DerivedCondition2 -> /world:DerivedCondition1 (124D33D097CF90D0)

not ideal, but this can help in the short term.

I was experimenting with something today which I also think might help, which was to change the way from LHCb etc. we interact with dd4hep, in that we pass all derived conditions to dd4hep as std::any objects. In this way, we then are able on the LHCb side to perform the type safety checks mostly lacking in dd4hep itself, as we just require it to store this single type, and we handle the type checks ourselves. I have not finished things yet, and its a 4 day holiday now in the UK but I will try and finish this up next week. its not the whole fix, but I think it will also help a bit.

@bcouturi I have just submitted LHCb!3607 (merged). It would be interesting to see if it helps here. In my testing, whilst tuning the logic I got the exceptions a few times, and they did their job in these cases, giving me info on both what type was expected, and actually saved in the condition payload.

b.t.w. I've added the above MR to lhcb-dd4hep so assuming it works it should be in this slots builds tomorrow.

mentioned in merge request LHCb!3607 (merged)

Will have a look. It may clash with LHCb!3604 (merged). We improved the exceptions thrown in case of problem with Marco this morning.

Thanks. Yes, it will conflict with the changes. For me the std::any is probably a better approach, or some merger of the two, as it allows us to know both the type expected and received types from dd4hep via the any type info. I also suspect it will indirectly solve a number of other issues, such as dd4hep memory alignment issue.

So, on a quick review I think there is good stuff in each MR such that probably the best is to merge the two. It will need to be done by hand, but I think it perfectly possible to do, and I think probably the best going forward.

In that case, would you mind if I merge LHCb!3604 (merged) and you rebase/merge LHCb!3607 (merged) ? Sorry about that, it's just me being lazy as a maintainer as LHCb!3604 (merged) is tested and teady to be merged while LHCb!3607 (merged) failed to merge into dd4hep slot and thus needs a rebase anyway

Sure thats fine.

Note though LHCb!3607 (merged) should be clean against master. I think the issue above is it conflicts with the changes in LHCb!3553 (merged). Could you look to merging that one as well as LHCb!3604 (merged) @sponce as then I can just rebase against both at the same time.

Great. Thanks ! I've just merged LHCb!3604 (merged). And as expected LHCb!3607 (merged) has a merge conflict.

can you merge LHCb!3553 (merged) as well so I can rebase against both ?

b.t.w. it probably won't be before Monday now before I get to do the rebasing.

Ok, I'll try to deal with LHCb!3553 (merged) in the mean time. It's failing tests in Alignment at the moment

Done, LHCb!3553 (merged) has been merged.

Thanks. I'l been off ill the last couple of days but will be looking to this today hopefully.

changed the description

changed title from Unknown missing dd4hep condition to PrStoreUTHit - Problems loading dd4hep condition

assigned to @bcouturi

@clemenci @bcouturi @sponce Has there been any further understanding of what is going wrong here with PrStoreUThits and how to address it ?

mentioned in issue #349 (closed)

mentioned in merge request !2740 (merged)

closed with merge request !2740 (merged)

mentioned in commit ed160320

mentioned in commit abae11c2