Use "best matching" EDAlias in module dependence checks when placing …

…ConditionalTask modules in Path

If the EDAlias has wildcards for one module, and more specific aliases
for another module, we should use the one that has the most
constrained match. There is one ambiguous case left, for which an
exception is thrown. I hope we won't encounter that situation in
practice.

The problem was encountered with a SwitchProducer having the EDAlias
case, but I think the problem could happen also without
SwitchProducer.

FWCore/Framework/src/StreamSchedule.cc

@@ -35,6 +35,7 @@
 #include <cstdlib>
 #include <functional>
 #include <iomanip>
+#include <limits>
 #include <list>
 #include <map>
 #include <exception>
@@ -159,6 +160,74 @@ namespace edm {
       }
       return std::pair(beg + 1, std::prev(modnames.end()));
     }
+
+    std::optional<std::string> findBestMatchingAlias(
+        std::unordered_multimap<std::string, edm::BranchDescription const*> const& conditionalModuleBranches,
+        std::unordered_multimap<std::string, StreamSchedule::AliasInfo> const& aliasMap,
+        std::string const& productModuleLabel,
+        ConsumesInfo const& consumesInfo) {
+      std::optional<std::string> best;
+      int wildcardsInBest = std::numeric_limits<int>::max();
+      bool bestIsAmbiguous = false;
+
+      auto updateBest = [&best, &wildcardsInBest, &bestIsAmbiguous](
+                            std::string const& label, bool instanceIsWildcard, bool typeIsWildcard) {
+        int const wildcards = static_cast<int>(instanceIsWildcard) + static_cast<int>(typeIsWildcard);
+        if (wildcards == 0) {
+          bestIsAmbiguous = false;
+          return true;
+        }
+        if (not best or wildcards < wildcardsInBest) {
+          best = label;
+          wildcardsInBest = wildcards;
+          bestIsAmbiguous = false;
+        } else if (best and *best != label and wildcardsInBest == wildcards) {
+          bestIsAmbiguous = true;
+        }
+        return false;
+      };
+
+      auto findAlias = aliasMap.equal_range(productModuleLabel);
+      for (auto it = findAlias.first; it != findAlias.second; ++it) {
+        std::string const& aliasInstanceLabel =
+            it->second.instanceLabel != "*" ? it->second.instanceLabel : it->second.originalInstanceLabel;
+        bool const instanceIsWildcard = (aliasInstanceLabel == "*");
+        if (instanceIsWildcard or consumesInfo.instance() == aliasInstanceLabel) {
+          bool const typeIsWildcard = it->second.friendlyClassName == "*";
+          if (typeIsWildcard or (consumesInfo.type().friendlyClassName() == it->second.friendlyClassName)) {
+            if (updateBest(it->second.originalModuleLabel, instanceIsWildcard, typeIsWildcard)) {
+              return it->second.originalModuleLabel;
+            }
+          } else if (consumesInfo.kindOfType() == ELEMENT_TYPE) {
+            //consume is a View so need to do more intrusive search
+            //find matching branches in module
+            auto branches = conditionalModuleBranches.equal_range(productModuleLabel);
+            for (auto itBranch = branches.first; itBranch != branches.second; ++it) {
+              if (typeIsWildcard or itBranch->second->productInstanceName() == it->second.originalInstanceLabel) {
+                if (productholderindexhelper::typeIsViewCompatible(consumesInfo.type(),
+                                                                   TypeID(itBranch->second->wrappedType().typeInfo()),
+                                                                   itBranch->second->className())) {
+                  if (updateBest(it->second.originalModuleLabel, instanceIsWildcard, typeIsWildcard)) {
+                    return it->second.originalModuleLabel;
+                  }
+                }
+              }
+            }
+          }
+        }
+      }
+      if (bestIsAmbiguous) {
+        throw Exception(errors::UnimplementedFeature)
+            << "Encountered ambiguity when trying to find a best-matching alias for\n"
+            << " friendly class name " << consumesInfo.type().friendlyClassName() << "\n"
+            << " module label " << productModuleLabel << "\n"
+            << " product instance name " << consumesInfo.instance() << "\n"
+            << "when processing EDAliases for modules in ConditionalTasks. Two aliases have the same number of "
+               "wildcards ("
+            << wildcardsInBest << ")";
+      }
+      return best;
+    }
   }  // namespace
 
   // -----------------------------
@@ -658,38 +727,11 @@ namespace edm {
           auto itFound = conditionalModules.find(productModuleLabel);
           if (itFound == conditionalModules.end()) {
             //Check to see if this was an alias
-            auto findAlias = aliasMap.equal_range(productModuleLabel);
-            for (auto it = findAlias.first; it != findAlias.second; ++it) {
-              //this was previously filtered so only the conditional modules remain
-              productModuleLabel = it->second.originalModuleLabel;
-              if (it->second.instanceLabel == "*" or ci.instance() == it->second.instanceLabel) {
-                if (it->second.friendlyClassName == "*" or
-                    (ci.type().friendlyClassName() == it->second.friendlyClassName)) {
-                  productFromConditionalModule = true;
-                  //need to check the rest of the data product info
-                  break;
-                } else if (ci.kindOfType() == ELEMENT_TYPE) {
-                  //consume is a View so need to do more intrusive search
-                  //find matching branches in module
-                  auto branches = conditionalModuleBranches.equal_range(productModuleLabel);
-                  for (auto itBranch = branches.first; itBranch != branches.second; ++it) {
-                    if (it->second.originalInstanceLabel == "*" or
-                        itBranch->second->productInstanceName() == it->second.originalInstanceLabel) {
-                      if (typeIsViewCompatible(ci.type(),
-                                               TypeID(itBranch->second->wrappedType().typeInfo()),
-                                               itBranch->second->className())) {
-                        productFromConditionalModule = true;
-                        break;
-                      }
-                    }
-                  }
-                  if (productFromConditionalModule) {
-                    break;
-                  }
-                }
-              }
-            }
-            if (productFromConditionalModule) {
+            //note that aliasMap was previously filtered so only the conditional modules remain there
+            auto foundAlias = findBestMatchingAlias(conditionalModuleBranches, aliasMap, productModuleLabel, ci);
+            if (foundAlias) {
+              productModuleLabel = *foundAlias;
+              productFromConditionalModule = true;
               itFound = conditionalModules.find(productModuleLabel);
               //check that the alias-for conditional module has not been used
               if (itFound == conditionalModules.end()) {

FWCore/Integration/test/run_TestSwitchProducer.sh

@@ -79,6 +79,19 @@ pushd ${LOCAL_TMP_DIR}
   echo "SwitchProducer in a ConditionalTask, test EDAlias with all cases being explicitly consumed, case test2 disabled"
   cmsRun -n ${NUMTHREADS} ${LOCAL_TEST_DIR}/${test}ConditionalTaskEDAliasConsumeAllCases_cfg.py disableTest2 || die "cmsRun ${test}ConditionalTaskEDAliasConsumeAllCases_cfg.py 2" $?
 
+  echo "*************************************************"
+  echo "SwitchProducer in a ConditionalTask, test EDAlias with a wildcard"
+  cmsRun -n ${NUMTHREADS} ${LOCAL_TEST_DIR}/${test}ConditionalTaskEDAliasWildcard_cfg.py || die "cmsRun ${test}ConditionalTaskEDAliasWildcard_cfg.py" $?
+
+  echo "*************************************************"
+  echo "SwitchProducer in a ConditionalTask, test EDAlias with a wildcard pattern, other module having the wildcard"
+  cmsRun -n ${NUMTHREADS} ${LOCAL_TEST_DIR}/${test}ConditionalTaskEDAliasWildcard_cfg.py -- --patternOnOtherModule || die "cmsRun ${test}ConditionalTaskEDAliasWildcard_cfg.py --patternOnOtherModule" $?
+
+  echo "*************************************************"
+  echo "SwitchProducer in a ConditionalTask, test EDAlias with an a wildcard, case test2 disabled"
+  cmsRun -n ${NUMTHREADS} ${LOCAL_TEST_DIR}/${test}ConditionalTaskEDAliasWildcard_cfg.py -- --disableTest2 || die "cmsRun ${test}ConditionalTaskEDAliasWildcard_cfg.py --disableTest2" $?
+
+
 
   echo "*************************************************"
   echo "SwitchProducer in a Path"

FWCore/Integration/test/testSwitchProducerConditionalTaskEDAliasWildcard_cfg.py

@@ -0,0 +1,86 @@
+import FWCore.ParameterSet.Config as cms
+import sys
+import argparse
+
+parser = argparse.ArgumentParser(prog=sys.argv[0], description="Test SwitchProducer, that has an EDAlias with '*' wildcard, in a ConditionalTask")
+
+parser.add_argument("--disableTest2", help="Disable 'test2' case of the SwitchProducerTest", action="store_true")
+parser.add_argument("--wildcardOnOtherModule", help="Use the wildcard for alias from another module", action="store_true")
+
+argv = sys.argv[:]
+if '--' in argv:
+    argv.remove("--")
+args, unknown = parser.parse_known_args(argv)
+
+enableTest2 = not args.disableTest2
+class SwitchProducerTest(cms.SwitchProducer):
+    def __init__(self, **kargs):
+        super(SwitchProducerTest,self).__init__(
+            dict(
+                test1 = lambda accelerators: (True, -10),
+                test2 = lambda accelerators: (enableTest2, -9)
+            ), **kargs)
+
+process = cms.Process("PROD1")
+
+process.source = cms.Source("EmptySource")
+
+process.maxEvents.input = 3
+
+process.intProducer1 = cms.EDProducer(
+    "ManyIntProducer",
+    ivalue = cms.int32(1),
+    values = cms.VPSet(
+    )
+)
+process.intProducer2 = cms.EDProducer(
+    "ManyIntProducer",
+    ivalue = cms.int32(11),
+    values = cms.VPSet(
+        cms.PSet(instance=cms.string("bar"), value=cms.int32(12))
+    )
+)
+process.intProducer3 = cms.EDProducer(
+    "ManyIntProducer",
+    ivalue = cms.int32(21)
+)
+if args.wildcardOnOtherModule:
+    process.intProducer1.values.append(cms.PSet(instance=cms.string("bar"), value=cms.int32(2)))
+    process.intProducer2.values = []
+
+process.intProducer4 = cms.EDProducer(
+    "ManyIntProducer",
+    ivalue = cms.int32(31),
+    values = cms.VPSet(
+        cms.PSet(instance=cms.string("foo"), value=cms.int32(32)),
+        cms.PSet(instance=cms.string("bar"), value=cms.int32(33)),
+        cms.PSet(instance=cms.string("xyzzy"), value=cms.int32(34)),
+   )
+)
+
+process.intProducer = SwitchProducerTest(
+    test1 = cms.EDAlias(
+        intProducer4 = cms.EDAlias.allProducts()
+    ),
+    test2 = cms.EDAlias()
+)
+allMatchName = "intProducer1"
+otherName ="intProducer2"
+if args.wildcardOnOtherModule:
+    (allMatchName, otherName) = (otherName, allMatchName)
+setattr(process.intProducer.test2, allMatchName, cms.EDAlias.allProducts())
+setattr(process.intProducer.test2, otherName, cms.VPSet(
+    cms.PSet(type = cms.string("*"), fromProductInstance = cms.string(""), toProductInstance = cms.string("foo")),
+    cms.PSet(type = cms.string("*"), fromProductInstance = cms.string("bar"), toProductInstance = cms.string("*"))
+))
+process.intProducer.test2.intProducer3 = cms.VPSet(
+    cms.PSet(type = cms.string("edmtestIntProduct"), toProductInstance = cms.string("xyzzy"))
+)
+
+process.intConsumer = cms.EDAnalyzer("edmtest::GenericIntsAnalyzer", srcEvent = cms.untracked.VInputTag("intProducer"))
+process.intConsumer2 = cms.EDAnalyzer("edmtest::GenericIntsAnalyzer", srcEvent = cms.untracked.VInputTag("intProducer", "intProducer2", "intProducer3"))
+
+process.ct = cms.ConditionalTask(process.intProducer1, process.intProducer2, process.intProducer3, process.intProducer4, process.intProducer)
+
+process.p1 = cms.Path(process.intConsumer, process.ct)
+process.p2 = cms.Path(process.intConsumer2, process.ct)