Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Alternative module configuration syntax #43955

Merged
merged 2 commits into from
Mar 3, 2024
Merged

Alternative module configuration syntax #43955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d9a62e0
Exclude keys with '@' from cfi generation
Dr15Jones Feb 13, 2024
acefa33
Alternative python module configuration syntax
Dr15Jones Feb 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions FWCore/Integration/test/check_empty_event_cfg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import FWCore.ParameterSet.Config as cms
from FWCore.Modules.modules import EventContentAnalyzer, EmptySource

process = cms.Process("EMPTY")

process.test = cms.EDAnalyzer("EventContentAnalyzer", listPathStatus = cms.untracked.bool(True))
process.test = EventContentAnalyzer(listPathStatus = True)

process.e = cms.EndPath(process.test)

#process.out = cms.OutputModule("AsciiOutputModule")
#process.e2 = cms.EndPath(process.out)

process.source = cms.Source("EmptySource")
process.source = EmptySource()

process.maxEvents.input = 1
24 changes: 14 additions & 10 deletions FWCore/Integration/test/delayedreader_throw_cfg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,31 @@
import FWCore.ParameterSet.Config as cms
from FWCore.Framework.modules import AddIntsProducer, IntProductFilter
from FWCore.Modules.modules import AsciiOutputModule
from FWCore.Integration.modules import DelayedReaderThrowingSource

process = cms.Process("TEST")

process.source = cms.Source("DelayedReaderThrowingSource", labels = cms.untracked.vstring("test", "test2", "test3"))
process.source = DelayedReaderThrowingSource( labels = ["test", "test2", "test3"])

process.getter = cms.EDProducer("AddIntsProducer", labels = cms.VInputTag(cms.InputTag("test","","INPUTTEST")))
process.onPath = cms.EDProducer("AddIntsProducer", labels = cms.VInputTag(cms.InputTag("test2", "", "INPUTTEST"), cms.InputTag("getter", "other")))
process.f1 = cms.EDFilter("IntProductFilter", label = cms.InputTag("onPath"), shouldProduce = cms.bool(True))
process.f2 = cms.EDFilter("IntProductFilter", label = cms.InputTag("onPath"), shouldProduce = cms.bool(True))
process.inFront = cms.EDFilter("IntProductFilter", label = cms.InputTag("test3"))
process.getter = AddIntsProducer(labels = [("test","","INPUTTEST")])
process.onPath = AddIntsProducer(labels = [("test2", "", "INPUTTEST"), ("getter", "other")])
process.f1 = IntProductFilter( label = "onPath", shouldProduce = True)
process.f2 = IntProductFilter( label = "onPath", shouldProduce = True)
process.inFront = IntProductFilter( label = "test3")
Comment on lines -5 to +14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something I was thinking about ages ago, when I was spending too much of my time looking after the HLT configurations, and I'm glad to see it being implemented !

Looking at the example, the main concern I have is for InputTags and VInputTags: writing or reading

labels = [("test2", "", "INPUTTEST"), ("getter", "other")]

I think it's very easy to get confused about getter and other being two components on an InputTag instead of two separate InputTags.

Some other confusing exmples:

this is a VInputTag with only one InputTag ?

labels = [("getter", "other")]

this is a VInputTag with two InputTags ?

labels = ["getter", "other"]

this is a VInputTag with two InputTags ?

labels = ["getter"]

this is a VInputTag with two InputTags ?

labels = [("getter")]

this is a VInputTag with two InputTags ?

labels = [("getter",)]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fwyzard Thanks for the feedback! All the examples are already possible since this code just reuses what is already available. The reduced syntax can already be used in

  • Modifiers
  • clone statements
  • explicit changing of existing established parameters

E.g.

>>> import FWCore.ParameterSet.Config as cms
>>> test = cms.EDProducer("FOO", srcs = cms.VInputTag())
>>> test.srcs = ["getter"]
>>> print(test.dumpPython())
cms.EDProducer("FOO",
    srcs = cms.VInputTag("getter")
)

That being said, the reduced syntax also excepts the explicit type information as well

>>> test.srcs = [cms.InputTag("getter")]
>>> print(test.dumpPython())
cms.EDProducer("FOO",
    srcs = cms.VInputTag(cms.InputTag("getter"))
)

As for some of your explicit examples, I'm curious why you ask if the following are VInputTags with two entries, rather than (the correct) a VInputTags with 1 entry.

labels = ["getter"]
labels = [("getter")]

or even

labels = [("getter",)]

which does require one to understand that (...,) in python is just a way to express a tuple with a single entry.

All the above are standard python for a container with 1 entry.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two cases could be interpreted as ambiguous for understanding the type of labels (InputTag vs VInputTag) only from the assignment.

this is a VInputTag with two InputTags ?

labels = ["getter", "other"]

The same syntax would work for InputTag (with module and instance labels) and for 2-element VInputTag

this is a VInputTag with two InputTags ?

labels = ["getter"]

The same syntax would work for InputTag (with module labels) and for 1-element VInputTag.

(for reference, the sequence-syntax for assining InputTag was added in #29992, #29846)


process.p1 = cms.Path(process.inFront+process.onPath+process.f1+process.f2)
process.p3 = cms.Path(process.onPath+process.f1, cms.Task(process.getter))

process.p2 = cms.Path(process.onPath+process.f2)


#process.dump = cms.EDAnalyzer("EventContentAnalyzer")
#from FWCore.Modules.modules import EventContentAnalyzer import *
#process.dump = EventContentAnalyzer()
#process.p = cms.Path(process.dump)

process.out = cms.OutputModule("AsciiOutputModule")
process.out = AsciiOutputModule()
process.e = cms.EndPath(process.out, cms.Task(process.getter))

process.maxEvents.input = 1

#process.add_(cms.Service("Tracer"))
#from FWCore.Services.modules import Tracer
#process.add_(Tracer())
19 changes: 19 additions & 0 deletions FWCore/ParameterSet/bin/edmWriteConfigs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@
#include <utility>
#include "FWCore/Utilities/interface/Signal.h"
#include <sstream>
#include <fstream>
#include <filesystem>

static char const* const kHelpOpt = "help";
static char const* const kHelpCommandOpt = "help,h";
Expand Down Expand Up @@ -133,6 +135,20 @@ namespace {
pluginNames.push_back(nameAndType.first);
}
}

void writeModulesFile() {
if (std::filesystem::exists(std::filesystem::current_path() / "modules.py"))
return;
std::array<char, L_tmpnam> buffer;
std::tmpnam(buffer.data());
std::ofstream file{buffer.data()};

file << "from FWCore.ParameterSet.ModulesProxy import _setupProxies\n"
"locals().update(_setupProxies(__file__))\n";
file.close();
std::filesystem::copy(buffer.data(), "modules.py");
std::filesystem::remove(buffer.data());
}
} // namespace

int main(int argc, char** argv) try {
Expand Down Expand Up @@ -287,6 +303,9 @@ int main(int argc, char** argv) try {

std::set<std::string> usedCfiFileNames;
edm::for_all(pluginNames, std::bind(&writeCfisForPlugin, _1, factory, std::ref(usedCfiFileNames)));
if (not pluginNames.empty()) {
writeModulesFile();
}
});
} catch (cms::Exception& iException) {
if (!library.empty()) {
Expand Down
2 changes: 2 additions & 0 deletions FWCore/ParameterSet/interface/ConfigurationDescriptions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ namespace edm {
std::string const& pluginName,
std::set<std::string>& usedCfiFileNames);

void writeClassFile(ParameterSetDescription const&) const;

void printForLabel(std::pair<std::string, ParameterSetDescription> const& labelAndDesc,
std::ostream& os,
std::string const& moduleLabel,
Expand Down
24 changes: 24 additions & 0 deletions FWCore/ParameterSet/python/ModulesProxy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import os
from os.path import sep, join
import importlib

class _ModuleProxy (object):
def __init__(self, package, name):
self._package = package
self._name = name
self._caller = None
def __call__(self,**kwargs):
if not self._caller:
self._caller = getattr(importlib.import_module(self._package+'.'+self._name),self._name)
return self._caller(**kwargs)


def _setupProxies(fullName):
_cmssw_package_name='.'.join(fullName.split(sep)[-3:-1])
basename = fullName.split(sep)[-1]
pathname = fullName[:-1*len(basename)]
proxies = dict()
for filename in ( x for x in os.listdir(pathname) if (len(x) > 3 and x[-3:] == '.py' and x != basename and ((len(x) < 6) or (x[-6:] != 'cfi.py')))):
name = filename[:-3]
proxies[name] = _ModuleProxy(_cmssw_package_name, name)
return proxies
48 changes: 48 additions & 0 deletions FWCore/ParameterSet/src/ConfigurationDescriptions.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,54 @@ namespace edm {
std::cref(baseType_),
std::cref(pluginName_),
std::ref(usedCfiFileNames)));
if (defaultDescDefined_) {
writeClassFile(defaultDesc_);
} else if (descriptions_.size() == 1) {
writeClassFile(descriptions_.begin()->second);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean if the ConfigurationDescriptions contains many ParameterSetDescriptions of which none is the default (i.e. added with ParameterSetDescription::addDefault()), class python file is not created?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is correct.

}

void ConfigurationDescriptions::writeClassFile(ParameterSetDescription const& iDesc) const {
std::string pluginName = pluginName_;
auto found = pluginName.find("::");
while (found != std::string::npos) {
pluginName.replace(found, 2, "_");
found = pluginName.find("::");
}
//Symbols that can appear in our plugin names but can't in python function names
const std::string toReplace("@<>,");
found = pluginName.find_first_of(toReplace);
while (found != std::string::npos) {
pluginName.replace(found, 1, "_");
found = pluginName.find_first_of(toReplace, found);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC this means that a plugin with a name FooProducer@alpaka would lead to a generating function with the name FooProducer_alpaka. We may want to think about this more in the future (e.g. whether to drop the _alpaka part).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is how the name would be modified.


std::string fileName = pluginName + ".py";
std::ofstream outFile(fileName.c_str());
if (outFile.fail()) {
edm::Exception ex(edm::errors::LogicError, "Creating class file failed.\n");
ex << "Opening a file '" << fileName << "' failed.\n";
ex << "Error code from errno " << errno << ": " << std::strerror(errno) << "\n";

ex.addContext("Executing function ConfigurationDescriptions::writeDefault");
throw ex;
}
outFile << "import FWCore.ParameterSet.Config as cms\n\n";
outFile << "def " << pluginName
<< "(**kwargs):\n"
" mod = cms."
<< baseType_ << "('" << pluginName_ << "'";

bool startWithComma = true;
int indentation = 4;
iDesc.writeCfi(outFile, startWithComma, indentation);

outFile << ")\n"
" for k,v in kwargs.items():\n"
" setattr(mod, k, v)\n"
" return mod\n";

outFile.close();
}

void ConfigurationDescriptions::writeCfiForLabel(std::pair<std::string, ParameterSetDescription> const& labelAndDesc,
Expand Down
3 changes: 3 additions & 0 deletions FWCore/ParameterSet/src/ParameterDescriptionBase.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ namespace edm {

void ParameterDescriptionBase::writeCfi_(
std::ostream& os, bool optional, bool& startWithComma, int indentation, bool& wroteSomething) const {
if (label().empty() or label()[0] == '@') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was related to SwitchProducer, and its use of @... parameters to convey information from python to C++, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

return;
}
wroteSomething = true;
if (startWithComma)
os << ",";
Expand Down