Project

General

Profile

Bug #14763

FindMany silently ignores associations to non-available products

Added by Gianluca Petrillo over 3 years ago. Updated over 3 years ago.

Status:
Closed
Priority:
Normal
Target version:
Start date:
12/05/2016
Due date:
% Done:

100%

Estimated time:
Spent time:
Scope:
Internal
Experiment:
-
SSI Package:
Duration:

Description

Bug #14760 describes a problem with an association between two LArSoft data products, "tracks" and "flashes".
The evidence is that a art::FindMany does not report "flashes" for "tracks" which do have "flashes" associated with them.
The reason is that the helper which helps FindMany will not add the "flash" when its art pointer can't be resolved (as in the case linked above), and helpfully tell nobody about that.
Even more surprising, that seems to be true for art::FindManyP too.

I suppose this is the intended behaviour, because it's coded there, black on light gray in source:canvas/Persistency/Common/detail/IPRHelper.h line 371. Well, kind of. Is this from a specific user request or design choice?
After spending 4 hours tracking it down, it came to feel to me as a questionable choice, and I wonder if there is room to, in fact, question it.
For example, FindMany might throw an exception art::errors::what_are_you_doing and FindManyP would just add the dangling art pointer.


Related issues

Related to LArSoft - Bug #14760: Association creation/loading in uboonecode v06_16Closed12/05/2016

Is duplicate of canvas - Feature #14801: Association query objects should not ignore non-dereferenceable pointers given as inputClosed12/09/2016

Associated revisions

Revision 7a305923 (diff)
Added by Chris Green over 3 years ago

Provide consistent intended behavior per issue #14763.

History

#1 Updated by Gianluca Petrillo over 3 years ago

Sorry for the title... it was written when I still did not know what was happening.
Could a manager change it into something sensible, like "FindMany silently ignores associations to non-available products"?

#2 Updated by Gianluca Petrillo over 3 years ago

  • Related to Bug #14760: Association creation/loading in uboonecode v06_16 added

#3 Updated by Kyle Knoepfel over 3 years ago

  • Subject changed from FindMany can't find associated objects to FindMany silently ignores associations to non-available products

#4 Updated by Christopher Green over 3 years ago

This behavior is intentional, and is a consequence of the possibility that the provided reference collection may be provided in terms of pointers rather than exclusively Ptr.

In order to not be unfeasibly slow, an unordered_map lookup table is constructed from the Assns which is hashed by pointer to reference item type: this lookup table can only be filled with information from resolvable Ptr (i.e. available products). We then go through each provided item in the reference collection, pulling matches out of the lookup table. Since it is perfectly permissible to have an Assns with each "side" having Ptr into multiple products, not all of which are available, and the reference collection may (perhaps by construction) never refer to unavailable objects, it was felt to be unreasonable to require that all products used in an Assns be available in order to use it at all in the construction of a FindXXX. In summary, if one is constructing (e.g.) a FindOne<B> using an arbitrary collection referring to A in some way:
  1. The lookup table is constructed whose keys consist only of pointers to A from resolvable Ptr<A>. If the Assns has an unresolvable Ptr<A>, it is omitted from the lookup table.
  2. The reference collection is looked up in the table. If entries are found, what happens to the resulting Ptr<B> depends on whether we have a FindXXX or a FindXXXP:
    1. For FindXXXP, the Ptr<B> is included in the FindXXXP results, and its product's availability is not verified.
    2. For FindXXX, the resolved pointer is included in the FindXXXP results if available, and nullptr if not.

It is feasible to implement a fail-on-unavailable-Ptr<A>, but I'm not sure it should be the default behavior. It is possible to modify this to only worry about unavailable products that actually affect the result in the case where the reference collection is provided in terms of Ptr<A> or Handle<A>, but not if the reference collection is provided in terms of pointer-to-A. I think this issue would benefit from a discussion between interested parties before any action (including possibly no action) is decided upon.

#5 Updated by Gianluca Petrillo over 3 years ago

Christopher Green wrote:

  1. The lookup table is constructed whose keys consist only of pointers to A from resolvable Ptr<A>. If the Assns has an unresolvable Ptr<A>, it is omitted from the lookup table.
  2. The reference collection is looked up in the table. If entries are found, what happens to the resulting Ptr<B> depends on whether we have a FindXXX or a FindXXXP:
    1. For FindXXXP, the Ptr<B> is included in the FindXXXP results, and its product's availability is not verified.
    2. For FindXXX, the resolved pointer is included in the FindXXXP results if available, and nullptr if not.

The last description conflicts with my observation, in that the reported collection was empty, rather than having null pointers or dangling art pointers. I would infer your description from the code if the condition in line 371 (linked above) were always true.
Do you have a test that shows the behaviour?

#6 Updated by Christopher Green over 3 years ago

We do not currently have a test for the described behavior, but I will certainly put one together. However, I would emphasize that the behavior depends on whether the missing collection is (for, say, a FindMany<B>) a collection of A or B; if the former, then the FindMany<B> will indeed have no entries for any of its A reference items because, as I say in my previous comment, "If the Assns has an unresolvable Ptr<A>, it is omitted from the lookup table." If it is a collection of B that is missing from the event, then the FindMany<B> will have nullptr entries for applicable reference A entries.

#7 Updated by Christopher Green over 3 years ago

My apologies: it appears there is inconsistency in the different art::detail::BcollHelper::fill<>() functions, with my faulty idea of what should be happening, and between the different fill() functions.

explicit art::Ptr<T>::operator bool() returns isNonNull() && isAvailable(), thereby subverting the expected logic.

There are four fill() functions, each with their little, "quirks":

  1. For FindOne
    If Bcoll[index] is non-null, then an exception is thrown (many-to-many). The check does not distinguish currently between the case of no existing association and that of an existing association where the Ptr<B> did not resolve.
  2. For FindOneP
    If Bcoll[index] (a Ptr<B>) is non-null and available, then an exception is thrown (many-to-many), otherwise the Ptr<B> is placed into Bcoll at index. The check does not currently distinguish between the case of no existing association and that of an existing association where Ptr<B> is null or did not resolve.
  3. For FindMany
    If the Ptr<B> resolves, the item's pointer is pushed into the correct Bcoll[index].
  4. For FindManyP
    If the Ptr<B> resolves, the Ptr<B> is pushed into the correct Bcoll[index].

This is unfortunate. How to change the code to do something sensible and defensible should be discussed among interested parties.

#8 Updated by Marc Paterno over 3 years ago

  • Status changed from New to Feedback

We will request a meeting with Gianluca to discuss possible resolutions and desired program behavior.

#9 Updated by Christopher Green over 3 years ago

  • Related to Feature #14801: Association query objects should not ignore non-dereferenceable pointers given as input added

#10 Updated by Christopher Green over 3 years ago

  • Related to deleted (Feature #14801: Association query objects should not ignore non-dereferenceable pointers given as input)

#11 Updated by Christopher Green over 3 years ago

  • Has duplicate Feature #14801: Association query objects should not ignore non-dereferenceable pointers given as input added

#12 Updated by Christopher Green over 3 years ago

  • Has duplicate deleted (Feature #14801: Association query objects should not ignore non-dereferenceable pointers given as input)

#13 Updated by Christopher Green over 3 years ago

  • Is duplicate of Feature #14801: Association query objects should not ignore non-dereferenceable pointers given as input added

#14 Updated by Christopher Green over 3 years ago

  • Status changed from Feedback to Closed
  • Assignee set to Christopher Green
  • Target version set to 2.06.00
  • % Done changed from 0 to 100


Also available in: Atom PDF