Merge pull request #39409 from makortel/fixSwitchProducerEDAliasCondi…

…tionalTask

Use "best matching" EDAlias in module dependence checks when placing ConditionalTask modules in Path

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)