SelectivePacker: Consistently check whether TES entries are explicitly requested

SelectivePacker only explicitly checked whether KeyedContainers and RecSummary it had access to were explicitly requested -- other TES entries were added to the packed output regardless of any request, as soon as they existed in the TES. This MR moves this check 'upstream' in the code so that this is done in the same place for every type of object.

As a result, it may be that objects other than KeyedContainer and RecSummary are no longer implicitly copied, and the configuration may need to be adapted to explicitly copy them instead. To help with this, a new property 'AlwaysPack' has been added which specifies locations that should be packed regardless of any Hlt/Spruce decisions.

Must be applied in conjunction with Moore!2281 (merged) -- which also depends on !4149 (merged)

Event/EventPacker/src/component/SelectivePacker.cpp

@@ -58,6 +58,10 @@
 #include <numeric>
 #include <type_traits>
 
+namespace {
+  using DataObjIDMapper_t = std::map<DataObjID, DataObjID>;
+}
+
 namespace Gaudi::Parsers {
   StatusCode parse( std::map<std::string, DataObjIDColl>& result, const std::string& s ) {
     std::map<std::string, std::vector<std::string>> t;
@@ -71,6 +75,26 @@ namespace Gaudi::Parsers {
       }
     } );
   }
+
+  StatusCode parse( std::pair<DataObjID, DataObjID>& result, const std::pair<std::string, std::string>& kv ) {
+    DataObjID k;
+    DataObjID v;
+    return parse( k, kv.first ).andThen( [&] { return parse( v, kv.second ); } ).andThen( [&] { result = {k, v}; } );
+  }
+
+  StatusCode parse( DataObjIDMapper_t& result, const std::string& s ) {
+    std::map<std::string, std::string> t;
+    return parse( t, s ).andThen( [&] {
+      result.clear();
+      return std::accumulate( t.begin(), t.end(), StatusCode{StatusCode::SUCCESS},
+                              [&]( StatusCode sc, const auto& kv ) {
+                                return sc.andThen( [&] {
+                                  std::pair<DataObjID, DataObjID> p;
+                                  return parse( p, kv ).andThen( [&] { result.emplace( p ); } );
+                                } );
+                              } );
+    } );
+  }
 } // namespace Gaudi::Parsers
 
 namespace LHCb {
@@ -80,6 +104,7 @@ namespace LHCb {
   }
 
   namespace {
+
     template <typename C>
     auto size( C const& c ) -> decltype( c.size() ) {
       return c.size();
@@ -193,6 +218,7 @@ namespace LHCb {
 
     class Encoder {
       DataObjIDColl const*                          m_requested;
+      DataObjIDMapper_t const*                      m_external;
       IIndexedANNSvc::inv_map_t const*              m_map;
       std::unordered_map<std::string, unsigned int> m_deps;
       unsigned                                      m_key = 0;
@@ -213,9 +239,10 @@ namespace LHCb {
       struct Anonymize_tag;
       using Anonymize = LHCb::tagged_bool<Anonymize_tag>;
 
-      Encoder( DataObjIDColl const& requested, IIndexedANNSvc const& annsvc, unsigned key, std::string_view stream,
-               Anonymize anonymize )
-          : m_requested{&requested}
+      Encoder( DataObjIDColl const* requested, DataObjIDMapper_t const* external, IIndexedANNSvc const& annsvc,
+               unsigned key, std::string_view stream, Anonymize anonymize )
+          : m_requested{requested}
+          , m_external{external}
           , m_map{&annsvc.s2i( key, PackedObjectLocations )}
           , m_key{key}
           , m_prefix{fmt::format( "/Event/{}/", remove_suffix( remove_prefix( stream, "/Event/" ), "/" ) )}
@@ -230,6 +257,11 @@ namespace LHCb {
         return std::string{m_prefix}.append( remove_prefix( loc, "/Event/" ) );
       }
       auto operator()( std::string_view loc ) {
+        // if external, just do the minimal thing as instructed, and no more...
+        // also: external locations can (by construction!) not be anonymized!
+        if ( auto iext = m_external->find( DataObjID{std::string{loc}} ); iext != m_external->end() ) {
+          return id( iext->second.key() );
+        }
         auto packed = packedLocation( loc );
         if ( m_anonymize && m_requested->find( DataObjID{std::string{loc}} ) == m_requested->end() ) {
           // implicit dependency -- generate location ID for this location if not already present (for this event!)
@@ -332,9 +364,11 @@ namespace LHCb {
                         LHCb::Hlt::PackedData::PackedDataChecksum* checksum ) const {
           for ( auto i : m_set ) {
             if ( m_packer.parent().msgLevel( MSG::DEBUG ) ) {
-              m_packer.parent().debug() << "Location " << identifier( *i ) << " (" << encoder( identifier( *i ) ) << ")"
-                                        << " size " << size( *i ) << " version " << static_cast<int>( i->version() )
-                                        << " type " << System::typeinfoName( typeid( C ) ) << endmsg;
+              m_packer.parent().debug() << fmt::format( "Location {} ( -> 0x{:08x} ) size {} version {:x} type {}",
+                                                        identifier( *i ), encoder( identifier( *i ) ), size( *i ),
+                                                        static_cast<int>( i->version() ),
+                                                        System::typeinfoName( typeid( C ) ) )
+                                        << endmsg;
             }
             const auto pdata = create_packeddata( *i );
             m_counters->nbPackedData += pdata->data().size();
@@ -669,7 +703,10 @@ namespace LHCb {
 
         Packer& packer() { return m_packer; }
 
-        bool emplace( Entry const* v ) { return m_set.emplace( *v ).second; }
+        bool emplace( Entry const* v ) {
+          assert( v != nullptr );
+          return m_set.emplace( *v ).second;
+        }
 
         void append_to( Buffer& buffer, Encoder& encoder, bool check,
                         LHCb::Hlt::PackedData::PackedDataChecksum* checksum ) const {
@@ -900,6 +937,8 @@ namespace LHCb {
 
       template <typename From, typename To, typename Weight>
       void add( const RelationWeighted1D<From, To, Weight>& r ) {
+        // FIXME: what if the table is empty???
+        // if (r.empty()) std::cerr << "FIXME: got an empty relation to pack..." << std::endl;
         auto* item = ( m_options.prune_relations /* ( r.registry() )*/ ? &m_lists.get<From>() : nullptr );
         // if we prune, have to make sure that, from now on, adding new `From` entries is considered an error
         if ( item ) item->lock();
@@ -915,9 +954,25 @@ namespace LHCb {
         }
       }
 
+      // this is the initial entry point
       template <typename Container>
       void add( VOC<Container*> const& c ) {
-        for ( const auto& i : c ) add( i );
+        const auto& p = m_lists.template get<Container>().packer().parent();
+        for ( auto const* i : c ) {
+          if ( !i ) continue;
+          auto const& id = identifier( *i );
+          if ( is_external( id ) ) {
+            if ( p.msgLevel( MSG::DEBUG ) )
+              p.debug() << "skipping " << id << " -- should be externally persisted" << endmsg;
+            continue;
+          }
+          if ( !is_explicitly_requested( id ) ) {
+            if ( p.msgLevel( MSG::DEBUG ) )
+              p.debug() << " skipping " << id << " -- not requested for this event" << endmsg;
+            continue;
+          }
+          add( *i );
+        }
       }
 
       template <typename V>
@@ -927,37 +982,40 @@ namespace LHCb {
 
       template <typename V, typename M>
       void add( const KeyedContainer<V, M>& kc ) {
-        auto r = kc.registry();
-        if ( r && is_explicitly_requested( identifier( *r ) ) ) {
-          for ( const auto& i : kc ) add( i );
-        } else {
-          // std::cerr << "skipped " << identifier( kc ) << std::endl; TODO: add  a counter for each input location, and
-          // count how often it is requested...
-        }
+        // FIXME: what if the container is empty???
+        // if (kc.empty()) std::cerr << "FIXME: requested to pack empty container: " << identifier(kc) << std::endl;
+        for ( const auto& i : kc ) add( i );
       }
 
-      void add( const RecSummary& rs ) {
-        auto r = rs.registry();
-        if ( r && is_explicitly_requested( identifier( *r ) ) ) m_lists.add( rs );
-      }
+      void add( const RecSummary& rs ) { m_lists.add( rs ); }
 
-      void add( const MCParticle& /*p*/ ) {
-        // foe now, assume MCParticles are somehow packed elsewhere...
+      void add( const MCParticle& p ) {
+        // for now, MCParticles should be packed externally...
+        if ( is_external( identifier( p ) ) ) return;
+        throw GaudiException( "Configuration error: reference to MCParticle in " + identifier( p ) + " will not work",
+                              __PRETTY_FUNCTION__, StatusCode::FAILURE );
+        // perhaps we should just pack it one day... just needs a trivial packer  -- the code already exists in
+        // PackMCParticle
 #if 0
-      if ( m_lists.add( p ) ) {
-        add( p.originVertex() );
-        add( p.endVertices() );
-      }
+        if ( m_lists.add( p ) ) {
+          add( p.originVertex() );
+          add( p.endVertices() );
+        }
 #endif
       }
 
-      void add( const MCVertex& /*v*/ ) {
-        // foe now, assume MCVertices are somehow packed elsewhere...
+      void add( const MCVertex& v ) {
+        // for now, MCVertices should be packed externally...
+        if ( is_external( identifier( v ) ) ) return;
+        throw GaudiException( "Configuration error: reference to MCVertex in " + identifier( v ) + " will not work",
+                              __PRETTY_FUNCTION__, StatusCode::FAILURE );
+        // perhaps we should just pack it one day... just needs a trivial packer  -- the code already exists in
+        // PackMCVertex
 #if 0
-      if ( m_lists.add( v ) ) {
-        add( v.mother() );
-        add( v.products() );
-      }
+        if ( m_lists.add( v ) ) {
+          add( v.mother() );
+          add( v.products() );
+        }
 #endif
       }
 
@@ -975,9 +1033,12 @@ namespace LHCb {
       };
 
       template <typename... Items>
-      ShoppingList( Gaudi::Algorithm const& parent, DataObjIDColl containers, CountersFor<Items...>& counters,
-                    Options options, VOC<Items*> const&... items )
-          : m_requested{std::move( containers )}, m_options{std::move( options )}, m_lists{parent, counters} {
+      ShoppingList( Gaudi::Algorithm const& parent, DataObjIDColl containers, DataObjIDMapper_t external,
+                    CountersFor<Items...>& counters, Options options, VOC<Items*> const&... items )
+          : m_requested{std::move( containers )}
+          , m_external{std::move( external )}
+          , m_options{std::move( options )}
+          , m_lists{parent, counters} {
         m_lists.get<CaloHypos>()
             .packer()
             .packDigitRefs( m_options.add_calo_digits )
@@ -990,6 +1051,7 @@ namespace LHCb {
         // TODO: verify that anything in containers is actually added in the end....
       }
 
+      bool is_external( DataObjID const& id ) { return m_external.find( id ) != m_external.end(); }
       bool is_explicitly_requested( DataObjID const& id ) { return m_requested.find( id ) != m_requested.end(); }
 
       template <typename Fun>
@@ -998,8 +1060,10 @@ namespace LHCb {
       }
 
     private:
-      DataObjIDColl m_requested = {};
-      Options       m_options   = {};
+      DataObjIDColl     m_requested = {}; // explicitly requested containers to be packed for this specific event
+      DataObjIDMapper_t m_external  = {}; // persisted 'out-of-bound' -- skip, but allow non-anynomous references into
+                                          // these
+      Options m_options = {};
       // list of all supported types -- i.e. _containers_ for which packers exist.
       Maps<Particles, Vertices, RecVertices, TwoProngVertices, ProtoParticles, Tracks, RichPIDs, MuonPIDs, CaloHypos,
            CaloClusters, CaloDigits, CaloAdcs, FlavourTags, WeightsVectors, /* MCParticles, MCVertices,*/ RecSummary,
@@ -1018,16 +1082,21 @@ namespace LHCb {
   class SelectivePacker final : public baseclass_t<PACKER...> {
     DataObjectReadHandle<HltDecReports>                   m_decrep{this, "DecReports", HltDecReportsLocation::Default};
     Gaudi::Property<std::map<std::string, DataObjIDColl>> m_map{this, "LineToLocations", {}};
-    Gaudi::Property<unsigned int>                         m_encodingKey{this, "EncodingKey", 0u};
-    Gaudi::Property<bool>                                 m_enableCheck{this, "EnableCheck", false};
-    Gaudi::Property<bool>                                 m_enableChecksum{this, "EnableChecksum", false};
-    Gaudi::Property<bool>                                 m_anonymizeDependencies{this, "AnonymizeDependencies", false};
-    Gaudi::Property<std::vector<std::string>>             m_addTrackAncestors{this, "AddTrackAncestors", {}};
-    Gaudi::Property<std::vector<std::string>>             m_addCaloDigits{this, "AddCaloDigits", {}};
-    Gaudi::Property<std::vector<std::string>>             m_addCaloClusters{this, "AddCaloClusters", {}};
-    Gaudi::Property<std::vector<std::string>>             m_addTagParticles{this, "AddTagParticles", {}};
-    Gaudi::Property<std::vector<std::string>>             m_addPVTracks{this, "AddPVTracks", {}};
-    Gaudi::Property<std::string>                          m_stream{this, "OutputPrefix", {}};
+    Gaudi::Property<DataObjIDColl>                        m_alwaysPack{this, "AlwaysPack", {}};
+    Gaudi::Property<DataObjIDMapper_t>                    m_externalLocations{
+        this, "ExternalLocations", {}}; // externally packed locations may get 'renamed' when the external packer packs
+                                                           // them.
+    Gaudi::Property<unsigned int>             m_encodingKey{this, "EncodingKey", 0u};
+    Gaudi::Property<bool>                     m_enableCheck{this, "EnableCheck", false};
+    Gaudi::Property<bool>                     m_enableChecksum{this, "EnableChecksum", false};
+    Gaudi::Property<bool>                     m_anonymizeDependencies{this, "AnonymizeDependencies", false};
+    Gaudi::Property<bool>                     m_awol_allowed{this, "AbsentRequestedContainerAllowed", false};
+    Gaudi::Property<std::vector<std::string>> m_addTrackAncestors{this, "AddTrackAncestors", {}};
+    Gaudi::Property<std::vector<std::string>> m_addCaloDigits{this, "AddCaloDigits", {}};
+    Gaudi::Property<std::vector<std::string>> m_addCaloClusters{this, "AddCaloClusters", {}};
+    Gaudi::Property<std::vector<std::string>> m_addTagParticles{this, "AddTagParticles", {}};
+    Gaudi::Property<std::vector<std::string>> m_addPVTracks{this, "AddPVTracks", {}};
+    Gaudi::Property<std::string>              m_stream{this, "OutputPrefix", {}};
     ServiceHandle<IIndexedANNSvc> m_annsvc{this, "ANNSvc", "HltANNSvc", "Service to retrieve PackedObjectLocations"};
 
     mutable CountersFor<typename PACKER::DataVector...> m_counters{this};
@@ -1039,7 +1108,7 @@ namespace LHCb {
     Buffer operator()( arg_for_packer_t<PACKER>... items ) const override {
       const auto& dec = m_decrep.get();
       // figure out the subset of containers to actually persist
-      DataObjIDColl containers;
+      DataObjIDColl containers = m_alwaysPack;
       if ( this->msgLevel( MSG::DEBUG ) ) this->debug() << "got positive decisions: ";
       for ( auto const& [k, dr] : *dec ) {
         if ( !dr.decision() ) continue;
@@ -1074,7 +1143,22 @@ namespace LHCb {
           },
           0, items... );
 
+      if ( !containers.empty() ) {
+        auto& log = ( m_awol_allowed ? this->warning() : this->error() );
+        for ( auto const& i : containers ) {
+          log << "requested container " << i << " not amongst configured inputs" << endmsg;
+        }
+        if ( !m_awol_allowed ) {
+          throw GaudiException( "Configuration error: requested container not amongst configured inputs",
+                                __PRETTY_FUNCTION__, StatusCode::FAILURE );
+        }
+      }
+
       if ( this->msgLevel( MSG::DEBUG ) ) {
+        this->debug() << "assuming the following containers are persisted through other means: ";
+        for ( const auto& i : m_externalLocations.value() ) this->debug() << i << " ";
+        this->debug() << endmsg;
+
         this->debug() << "containers available for this event: "; // TODO: for each container, report which
                                                                   // lines asked for it...
         for ( const auto& i : available ) this->debug() << i << " ";
@@ -1111,7 +1195,10 @@ namespace LHCb {
 #else
 #  pragma GCC diagnostic ignored "-Wpedantic"
 #endif
-      ShoppingList map{*this, requested, m_counters,
+      ShoppingList map{*this,
+                       requested,
+                       m_externalLocations,
+                       m_counters,
                        ShoppingList::Options{.add_track_ancestors  = any_fired( m_addTrackAncestors ),
                                              .add_calo_digits      = any_fired( m_addCaloDigits ),
                                              .add_calo_clusters    = any_fired( m_addCaloClusters ),
@@ -1125,8 +1212,9 @@ namespace LHCb {
       std::optional<LHCb::Hlt::PackedData::PackedDataChecksum> checksum;
       if ( m_enableChecksum ) checksum.emplace();
 
-      auto encoder =
-          Encoder{requested, *m_annsvc, m_encodingKey, m_stream, Encoder::Anonymize{m_anonymizeDependencies.value()}};
+      auto encoder = Encoder{&requested, &m_externalLocations.value(),
+                             *m_annsvc,  m_encodingKey,
+                             m_stream,   Encoder::Anonymize{m_anonymizeDependencies.value()}};
       map.for_each( [&, cksum = LHCb::get_pointer( checksum )]( const auto& c ) {
         c.append_to( buffer, encoder, m_enableCheck, cksum );
       } );
@@ -1155,7 +1243,10 @@ namespace LHCb {
           for ( auto id : internal ) this->debug() << id << " ";
           if ( !external.empty() ) {
             this->debug() << " external: ";
-            for ( auto id : external ) this->debug() << id << " ";
+            for ( auto id : external ) {
+              this->debug() << id << " ";
+              if ( id & 0x80000000 ) this->debug() << "(anonymized: this should never happen!) ";
+            }
           }
           this->debug() << endmsg;
         }