numberOfThreads does not check for type

[bfonta]

In a python configuration file, the maximum number of CPU threads to be considered by CMSSW when running a series of modules is specified as follows:

process.options = cms.untracked.PSet(
    numberOfThreads = cms.untracked.uint32(8),
    ... )

If the user, by mistake specifies the following:

process.options = cms.untracked.PSet(
    numberOfThreads = cms.untracked.uint32(8),
    numberOfStreams = cms.untracked.int32(8)
    ... )

it is prompted with a reasonably looking error:

Exception Message:
Parameter "numberOfStreams" should be defined as an untracked uint32.

However, an equivalent message is not displayed when the mistake happens with numberOfThreads; in that case, the CMSSW modules are run with one thread/stream only (when numberOfStreams is not specified, it is equal to numberOfThreads).

[cmsbuild]

A new Issue was created by @b-fontana .

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

cms-bot commands are listed here

[makortel]

assign core

[cmsbuild]

New categories assigned: core

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

[makortel]

I can reproduce in 11_2_0_pre8 (it's a bit strange though).

Note that already for some time now the process.options PSet is pre-defined by the cms.Process constructor and you can do just

process = cms.Process("TEST")
process.options.numberOfThreads = 8
process.options.numberOfStreams = 8

[makortel]

Ok, what happens is the following. The process.options PSet (throwing the exception for numberOfStreams) is called from EventProcessor::init()

cmssw/FWCore/Framework/src/EventProcessor.cc

Lines 345 to 348 in fc0a40f

	// Validates the parameters in the 'options', 'maxEvents', 'maxLuminosityBlocks',
	// and 'maxSecondsUntilRampdown' top level parameter sets. Default values are also
	// set in here if the parameters were not explicitly set.
	validateTopLevelParameterSets(parameterSet.get());

The EventProcessor::init() is called by its constructor, which is called by main()

cmssw/FWCore/Framework/bin/cmsRun.cpp

Lines 284 to 288 in fc0a40f

	tbb::task_arena arena(nThreads);
	arena.execute([&]() {
	context = "Constructing the EventProcessor";
	EventProcessorWithSentry procTmp(
	std::make_unique<edm::EventProcessor>(processDesc, jobReportToken, edm::serviceregistry::kTokenOverrides));

but before that the process.options.numberOfThreads is accessed here

cmssw/FWCore/Framework/bin/cmsRun.cpp

Line 250 in fc0a40f

auto threadsInfo = threadOptions(*pset);

cmssw/FWCore/ParameterSet/src/ThreadsInfo.cc

Lines 17 to 19 in fc0a40f

	if (ops.existsAs<unsigned int>("numberOfThreads", false)) {
	threadsInfo.nThreads_ = ops.getUntrackedParameter<unsigned int>("numberOfThreads");
	}

and then set in the PSet according to the actual value used for the TBB thread pool here

cmssw/FWCore/Framework/bin/cmsRun.cpp

Lines 267 to 268 in fc0a40f

	// update the numberOfThreads and sizeOfStackForThreadsInKB in the "options" ParameterSet
	setThreadOptions(threadsInfo, *pset);

cmssw/FWCore/ParameterSet/src/ThreadsInfo.cc

Line 32 in fc0a40f

newOp.addUntrackedParameter<unsigned int>("numberOfThreads", threadsInfo.nThreads_);

Therefore, if the type of numberOfThreads is incorrect, that value is ignored, and the parameter value is overwritten with the default value.

@Dr15Jones Could we simply change the ops.existsAs() to ops.exists() in

cmssw/FWCore/ParameterSet/src/ThreadsInfo.cc

Lines 17 to 21 in fc0a40f

	if (ops.existsAs<unsigned int>("numberOfThreads", false)) {
	threadsInfo.nThreads_ = ops.getUntrackedParameter<unsigned int>("numberOfThreads");
	}
	if (ops.existsAs<unsigned int>("sizeOfStackForThreadsInKB", false)) {
	threadsInfo.stackSize_ = ops.getUntrackedParameter<unsigned int>("sizeOfStackForThreadsInKB");

(or do we even need them at all given that cms.Process pre-defines the process.options) ?

[makortel]

We got bitten again by this problem (more details in the description of #41347)

[makortel]

(or do we even need them at all given that cms.Process pre-defines the process.options) ?

To answer my own question, I'd say "yes" because we can't really prevent del process.options.numberOfThreads (and that does not sound unreasonable way to go back to default).

[makortel]

#41349 attempts to fix along the lines described in #32070 (comment)