FairRoot icon indicating copy to clipboard operation
FairRoot copied to clipboard

Issue adding FairLink pointing to an element of a non-persistent data branch

Open BSali opened this issue 3 years ago • 0 comments

Describe the bug If FairMultiLinkedData.fPersistanceCheck flag is set to kFALSE the FairLinkManager::IsIgnoreType(...) call in FairMultiLinkedData::InsertLink(...) is NOT being bypassed. This prevents one from setting or adding FairLinks pointing to members/elements of non-persistent data branches.

Reason for the bypass When a non-persistent data branch is registered via the FairRootManager, the data type of the non-persistent data branch member is added to the FairLinkManager::fIgnoreTypes (happens within the FairRootManager::RegisterImpl(...) function). This FairLinkManager::fIgnoreTypes is queried during the FairMultiLinkedData::InsertLink(FairLink link) whether or not the type of the FairLink link is to be ignored (via FairLinkManager::IsIgnoreType(type)). If ignored, the FairLink link will not be inserted.

One way to overcome this problem seems to be to add the type of the non-persistent data branch element to the FairLinkManager via FairLinkManager::AddIncludeType(type). However, all other data types need to be added to the FairLinkManager as well once this function was called, not just the type of the non-persistent data branch member. Otherwise these types would be ignored from that point onward and can not be linked to. This is because FairLinkManager::AddIncludeType(type) will clear FairLinkManager::fIgnoreTypes and then use FairLinkManager::fIgnoreTypes to store all the allowed types. Types that are not in FairLinkManager::fIgnoreTypes are then ignored. This, I feel, makes it unexpectedly 'difficult' to use the FairLinkManager::AddIncludeType(type). That is why I would (for now) go for a bypass of the FairLinkManager::IsIgnoreType(type) test in FairMultiLinkedData::InsertLink(FairLink link) utilizing the FairMultiLinkedData.fPersistanceCheck flag.

Expected behavior If FairMultiLinkedData.fPersistanceCheck flag is set to kFALSE the FairLinkManager::IsIgnoreType(...) call in FairMultiLinkedData::InsertLink(...) should be bypassed.

In addition it may be helpful to notify the user that his attempt to add a FairLink to an ignored type failed via a LOG(WARN) or at least a LOG(DEBUG) (using the FairLogger) in FairMultiLinkedData::InsertLink(...). Else it would fail silently.

Quick Fix Changing line 187 and following in FairMultiLinkedData.cxx:

if (FairLinkManager::Instance()->IsIgnoreType(link.GetType())) {
    return;
}

to

if (fPersistanceCheck && FairLinkManager::Instance()->IsIgnoreType(link.GetType())) {
   LOG(WARN) << "FairMultiLinkedData::InsertLink(FairLink link) failed. Linking to data of type " << link.GetType() << " is not possible (non-persistent data). SetPersistanceCheck(kFALSE) to allow to link to non-persistent data.";
   return;
}

should allow to link to non-persistent data branch members/elements if the FairMultiLinkedData.fPersistanceCheck is set appropriately. However, this 'workaround' like fix does not address the (in my eyes) unexpected behaviour ofFairLinkManager::AddIncludeType(type). When FairLinkManager::AddIncludeType(type) is called I would merely remove type from the FairLinkManager::fIgnoreTypes-set. This would be what I would have expected to happen (and would make the 'Quick Fix' redundant). But then again I do not know the motivation for FairLinkManager and how other people use it, so changing anything there may be more problematic than including the fPersistanceCheck test in FairMultiLinkedData::InsertLink(...).^^

BSali avatar Aug 18 '20 09:08 BSali