improve the SwitchProducer error message to suggest a solution

[fwyzard]

Currently, if two branches in a SwitchProducer produce a different set of products, the error message is of limited usefulness.

For example:

%MSG
----- Begin Fatal Exception 25-Aug-2020 12:32:04 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
Exception Message:
SwitchProducer hltEcalDigis has a case hltEcalDigis@cpu with a product BranchKey(EcalDCCHeaderBlocksSorted, hltEcalDigis@cpu, , HLT) that is not produced by the chosen case hltEcalDigis@cuda
----- End Fatal Exception -------------------------------------------------

or the other way around

%MSG
----- Begin Fatal Exception 25-Aug-2020 12:35:29 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
Exception Message:
SwitchProducer hltEcalDigis has a case hltEcalDigis@cuda that does not produce a product BranchKey(EcalDCCHeaderBlocksSorted, hltEcalDigis, , HLT) that is produced by the chosen case hltEcalDigis@cpu
----- End Fatal Exception -------------------------------------------------

Assuming that the extra product is not needed, in order to fix the problem one needs to find the list of all other products - including their type - and populate an EDAlias declaration.

The error message could be more helpful and provide this list.

Even better, the SwitchProducer could use a flag to automatically restrict the products to the common subset.

[fwyzard]

assign core

[cmsbuild]

New categories assigned: core

@Dr15Jones,@smuzaffar,@makortel you have been requested to review this Pull request/Issue and eventually sign? Thanks

[cmsbuild]

A new Issue was created by @fwyzard Andrea Bocci.

@Dr15Jones, @dpiparo, @silviodonato, @smuzaffar, @makortel, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[makortel]

Even better, the SwitchProducer could use a flag to automatically restrict the products to the common subset.

Given that the same outcome can be achieved by EDAliases (whose construction can be tedious as written in the description), such a flag could indeed make sense. I can't quickly think of any concerns on it. Perhaps something like

foo = cms.SwitchProcucerCUDA(
    cpu = cms.EDProducer("FooProducer", ...),
    cuda = cms.EDProducer("FooProducerCUDA", ...),
    restrictProductsToCommonSubset_ = cms.bool(True)
)

? I'd use an underscore in the parameter name to signify that it is not a case of the switch, and I think it should be tracked because it affects which products exist.

[makortel]

Even better, the SwitchProducer could use a flag to automatically restrict the products to the common subset.

Given that the same outcome can be achieved by EDAliases (whose construction can be tedious as written in the description), such a flag could indeed make sense. I can't quickly think of any concerns on it.

On a deeper look that would require constructing the case-producers before their SwitchProducer module (module construction order is currently unspecified), because the set of common products can be known only after the case producers are constructed, and the SwitchProducermodule itself needs that information to set up its own consumes() and produces(). I might find/craft a way to mess with those afterwards, but it has a risk of becoming messy.

Other hurdle is that the current code expects normal EDAliases to be set up first before setting up the aliases within SwitchProducer (the current alias system is somewhat limited, see #25957). That would leave changing the consumes and produces information for SwitchProducer modules as the only option for that route.

Thinking out loud, a possible simple way to work around a mismatch in the products without requiring a full blown EDAlias setup would be to add a parameter to ignore given branches, e.g. something along

foo = cms.SwitchProcucerCUDA(
    cpu = cms.EDProducer("FooProducer", ...),
    cuda = cms.EDProducer("FooProducerCUDA", ...),
    ignoreProducts_ = vms.VPSet(
        PSet(type = cms.string("<friendly class name>")),
        PSet(type = cms.string("<friendly class name>"), productInstance = cms.string("<instance name>")
    )
)

(with a corresponding improvement in the error message)

[fwyzard]

The hardest part is to figure out all the branch names that need to be aliased and/or ignored.

So, if the error message in case of a mismatch could give the full list of branches from both sides of the switch, that would already help a lot.

Rather than giving a list of branches to ignore, I think it would make more sense to give a list of branches to use.

[makortel]

Rather than giving a list of branches to ignore, I think it would make more sense to give a list of branches to use.

That is certainly doable as well.

I also began to wonder if a generic mechanism to keep/ignore (similar to OutputModule's outputCommands, at least functionally) could be beneficial (or confusing).

[makortel]

The exception message is improved in #31414. I'm fine with keeping this issue open until a more automated solution is found.

[makortel]

Rather than giving a list of branches to ignore, I think it would make more sense to give a list of branches to use.

That is certainly doable as well.

To think out loud, reversing the logic in #31230 (comment) would lead to something along

foo = cms.SwitchProcucerCUDA(
    cpu = cms.EDProducer("FooProducer", ...),
    cuda = cms.EDProducer("FooProducerCUDA", ...),
    restrictToProducts_ = vms.VPSet(
        PSet(type = cms.string("<friendly class name>")),
        PSet(type = cms.string("<friendly class name>"), productInstance = cms.string("<instance name>")
    )
)

[makortel]

Some parts of the annoyance (differences in transient products) were reduced with #40104. Given that SwitchProducer doesn't result as portable configuration as wished, the Alpaka-based system using ModuleTypeResolver (see #40383 and links therein), and the adoption of Alpaka-based modules, it is not worth of the effort to improve SwitchProducer further (in fact it could be removed later on).

[makortel]

+core

[makortel]

@cmsbuild, please close

[cmsbuild]

cms-bot internal usage

[cmsbuild]

This issue is fully signed and ready to be closed.