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.

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

Improve command-line argument support for Python configs in cmsRun and edm tools #42650

Merged
merged 32 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
4076c85
remove optional flag for config_file
kpedro88 Aug 9, 2023
2cb316a
only give the python args to python
kpedro88 Aug 9, 2023
739bcd6
custom parser to take all args after config_file.py (even args starti…
kpedro88 Aug 10, 2023
25de618
handle sys.exit(0) from python
kpedro88 Aug 10, 2023
3f3f5cf
code format
kpedro88 Aug 10, 2023
a6e25a6
handle command-line arguments to config
kpedro88 Aug 10, 2023
3a6fac9
propagate command-line argument handling to edmConfigSplit
kpedro88 Aug 10, 2023
bd47a8c
imp module is deprecated, copy approach from edmConfigDump
kpedro88 Aug 10, 2023
d36e23c
remove redundant tool
kpedro88 Aug 11, 2023
63f7578
add a utility function for common operation (loading config from file…
kpedro88 Aug 11, 2023
1251e91
handle command line args in edmConfigHash
kpedro88 Aug 12, 2023
7ad87ee
add tests
kpedro88 Aug 23, 2023
9f77a48
handle case w/ no python options (& code-format)
kpedro88 Aug 23, 2023
59e6132
fix framework tests
kpedro88 Aug 23, 2023
9adf546
allow any name for config file
kpedro88 Aug 31, 2023
2519f50
avoid unnecessary c-string conversions for pybind interface
kpedro88 Aug 31, 2023
423868a
document use of extra_style_parser
kpedro88 Aug 31, 2023
3b222f1
add -c option to restore ability to pass python code on command line
kpedro88 Sep 1, 2023
bdbdddd
fix remaining FWCore tests
kpedro88 Sep 1, 2023
5c19593
more involved test updates
kpedro88 Sep 1, 2023
3cbfecb
code format
kpedro88 Sep 1, 2023
e4e39db
add more tests
kpedro88 Sep 1, 2023
d7e5a7b
don't need separator anymore
kpedro88 Sep 15, 2023
98bc57d
remove more unnecessary separator usage
kpedro88 Sep 18, 2023
0cd0b5f
actual unit test fixes revealed by eliminating parse_known_args
kpedro88 Sep 18, 2023
14fcd4e
next round of code review
kpedro88 Sep 27, 2023
28ca0be
fix typo in test
kpedro88 Sep 27, 2023
52d1b2c
refactor to make parser a separate class
kpedro88 Oct 2, 2023
6e45c6a
move exception check, help case into parser class
kpedro88 Oct 2, 2023
902890a
code format
kpedro88 Oct 2, 2023
a48e55d
some interface improvements
kpedro88 Oct 2, 2023
9d49116
add parser tests using catch2
kpedro88 Oct 3, 2023
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
1 change: 1 addition & 0 deletions FWCore/Framework/BuildFile.xml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<use name="boost"/>
<use name="boost_program_options"/>
<use name="rootcore"/>
<use name="tbb"/>
<use name="DataFormats/Common"/>
Expand Down
155 changes: 59 additions & 96 deletions FWCore/Framework/bin/cmsRun.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ This is a generic main that can be used with any plugin and a
PSet script. See notes in EventProcessor.cpp for details about it.
----------------------------------------------------------------------*/

#include "FWCore/Framework/interface/CmsRunParser.h"
#include "FWCore/Framework/interface/EventProcessor.h"
#include "FWCore/Framework/interface/defaultCmsRunServices.h"
#include "FWCore/MessageLogger/interface/ExceptionMessages.h"
Expand Down Expand Up @@ -30,7 +31,6 @@ PSet script. See notes in EventProcessor.cpp for details about it.

#include "TError.h"

#include "boost/program_options.hpp"
#include "oneapi/tbb/task_arena.h"

#include <cstring>
Expand All @@ -41,24 +41,6 @@ PSet script. See notes in EventProcessor.cpp for details about it.
#include <string>
#include <vector>

//Command line parameters
static char const* const kParameterSetOpt = "parameter-set";
static char const* const kPythonOpt = "pythonOptions";
static char const* const kParameterSetCommandOpt = "parameter-set,p";
static char const* const kJobreportCommandOpt = "jobreport,j";
static char const* const kJobreportOpt = "jobreport";
static char const* const kEnableJobreportCommandOpt = "enablejobreport,e";
static const char* const kEnableJobreportOpt = "enablejobreport";
static char const* const kJobModeCommandOpt = "mode,m";
static char const* const kJobModeOpt = "mode";
static char const* const kNumberOfThreadsCommandOpt = "numThreads,n";
static char const* const kNumberOfThreadsOpt = "numThreads";
static char const* const kSizeOfStackForThreadCommandOpt = "sizeOfStackForThreadsInKB,s";
static char const* const kSizeOfStackForThreadOpt = "sizeOfStackForThreadsInKB";
static char const* const kHelpOpt = "help";
static char const* const kHelpCommandOpt = "help,h";
static char const* const kStrictOpt = "strict";

// -----------------------------------------------
namespace {
class EventProcessorWithSentry {
Expand Down Expand Up @@ -101,10 +83,9 @@ namespace {
private:
edm::EventProcessor* ep_;
};

} // namespace

int main(int argc, char* argv[]) {
int main(int argc, const char* argv[]) {
edm::TimingServiceBase::jobStarted();

int returnCode = 0;
Expand All @@ -116,7 +97,7 @@ int main(int argc, char* argv[]) {
edm::s_defaultSizeOfStackForThreadsInKB * 1024);
std::shared_ptr<edm::Presence> theMessageServicePresence;
std::unique_ptr<std::ofstream> jobReportStreamPtr;
std::shared_ptr<edm::serviceregistry::ServiceWrapper<edm::JobReport> > jobRep;
std::shared_ptr<edm::serviceregistry::ServiceWrapper<edm::JobReport>> jobRep;
EventProcessorWithSentry proc;

try {
Expand Down Expand Up @@ -145,74 +126,39 @@ int main(int argc, char* argv[]) {
std::shared_ptr<edm::Presence>(edm::PresenceFactory::get()->makePresence("SingleThreadMSPresence").release());

context = "Processing command line arguments";
std::string descString(argv[0]);
descString += " [options] [--";
descString += kParameterSetOpt;
descString += "] config_file \nAllowed options";
boost::program_options::options_description desc(descString);

// clang-format off
desc.add_options()(kHelpCommandOpt, "produce help message")(
kParameterSetCommandOpt, boost::program_options::value<std::string>(), "configuration file")(
kJobreportCommandOpt,
boost::program_options::value<std::string>(),
"file name to use for a job report file: default extension is .xml")(
kEnableJobreportCommandOpt, "enable job report files (if any) specified in configuration file")(
kJobModeCommandOpt,
boost::program_options::value<std::string>(),
"Job Mode for MessageLogger defaults - default mode is grid")(
kNumberOfThreadsCommandOpt,
boost::program_options::value<unsigned int>(),
"Number of threads to use in job (0 is use all CPUs)")(
kSizeOfStackForThreadCommandOpt,
boost::program_options::value<unsigned int>(),
"Size of stack in KB to use for extra threads (0 is use system default size)")(kStrictOpt, "strict parsing");
// clang-format on

// anything at the end will be ignored, and sent to python
boost::program_options::positional_options_description p;
p.add(kParameterSetOpt, 1).add(kPythonOpt, -1);

// This --fwk option is not used anymore, but I'm leaving it around as
// it might be useful again in the future for code development
// purposes. We originally used it when implementing the boost
// state machine code.
boost::program_options::options_description hidden("hidden options");
hidden.add_options()("fwk", "For use only by Framework Developers")(
kPythonOpt,
boost::program_options::value<std::vector<std::string> >(),
"options at the end to be passed to python");

boost::program_options::options_description all_options("All Options");
all_options.add(desc).add(hidden);

boost::program_options::variables_map vm;
try {
store(boost::program_options::command_line_parser(argc, argv).options(all_options).positional(p).run(), vm);
notify(vm);
} catch (boost::program_options::error const& iException) {
edm::LogAbsolute("CommandLineProcessing")
<< "cmsRun: Error while trying to process command line arguments:\n"
<< iException.what() << "\nFor usage and an options list, please do 'cmsRun --help'.";
return edm::errors::CommandLineProcessing;
}

if (vm.count(kHelpOpt)) {
std::cout << desc << std::endl;
if (!vm.count(kParameterSetOpt))
edm::CmsRunParser parser(argv[0]);

const auto& parserOutput = parser.parse(argc, argv);
//return with exit code from parser
if (edm::CmsRunParser::hasExit(parserOutput))
return edm::CmsRunParser::getExit(parserOutput);
auto vm = edm::CmsRunParser::getVM(parserOutput);

std::string cmdString;
std::string fileName;
if (vm.count(edm::CmsRunParser::kCmdOpt)) {
cmdString = vm[edm::CmsRunParser::kCmdOpt].as<std::string>();
if (vm.count(edm::CmsRunParser::kParameterSetOpt)) {
edm::LogAbsolute("CommandLineProcessing") << "cmsRun: Error while trying to process command line arguments:\n"
<< "cannot use '-c [command line input]' with 'config_file'\n"
<< "For usage and an options list, please do 'cmsRun --help'.";
edm::HaltMessageLogging();
return 0;
}

if (!vm.count(kParameterSetOpt)) {
return edm::errors::CommandLineProcessing;
}
} else if (!vm.count(edm::CmsRunParser::kParameterSetOpt)) {
edm::LogAbsolute("ConfigFileNotFound") << "cmsRun: No configuration file given.\n"
<< "For usage and an options list, please do 'cmsRun --help'.";
edm::HaltMessageLogging();
return edm::errors::ConfigFileNotFound;
} else
fileName = vm[edm::CmsRunParser::kParameterSetOpt].as<std::string>();
std::vector<std::string> pythonOptValues;
if (vm.count(edm::CmsRunParser::kPythonOpt)) {
pythonOptValues = vm[edm::CmsRunParser::kPythonOpt].as<std::vector<std::string>>();
}
std::string fileName(vm[kParameterSetOpt].as<std::string>());
pythonOptValues.insert(pythonOptValues.begin(), fileName);

if (vm.count(kStrictOpt)) {
if (vm.count(edm::CmsRunParser::kStrictOpt)) {
//edm::setStrictParsing(true);
edm::LogSystem("CommandLineProcessing") << "Strict configuration processing is now done from python";
}
Expand All @@ -221,9 +167,9 @@ int main(int argc, char* argv[]) {
// Decide whether to enable creation of job report xml file
// We do this first so any errors will be reported
std::string jobReportFile;
if (vm.count(kJobreportOpt)) {
jobReportFile = vm[kJobreportOpt].as<std::string>();
} else if (vm.count(kEnableJobreportOpt)) {
if (vm.count(edm::CmsRunParser::kJobreportOpt)) {
jobReportFile = vm[edm::CmsRunParser::kJobreportOpt].as<std::string>();
} else if (vm.count(edm::CmsRunParser::kEnableJobreportOpt)) {
jobReportFile = "FrameworkJobReport.xml";
}
jobReportStreamPtr = jobReportFile.empty() ? nullptr : std::make_unique<std::ofstream>(jobReportFile.c_str());
Expand All @@ -234,15 +180,32 @@ int main(int argc, char* argv[]) {
jobRep.reset(new edm::serviceregistry::ServiceWrapper<edm::JobReport>(std::move(jobRepPtr)));
edm::ServiceToken jobReportToken = edm::ServiceRegistry::createContaining(jobRep);

context = "Processing the python configuration file named ";
context += fileName;
if (!fileName.empty()) {
context = "Processing the python configuration file named ";
context += fileName;
} else {
context = "Processing the python configuration from command line ";
context += cmdString;
}
std::shared_ptr<edm::ProcessDesc> processDesc;
try {
std::unique_ptr<edm::ParameterSet> parameterSet = edm::readConfig(fileName, argc, argv);
processDesc.reset(new edm::ProcessDesc(std::move(parameterSet)));
std::unique_ptr<edm::ParameterSet> parameterSet;
if (!fileName.empty())
parameterSet = edm::readConfig(fileName, pythonOptValues);
else
edm::makeParameterSets(cmdString, parameterSet);
processDesc = std::make_shared<edm::ProcessDesc>(std::move(parameterSet));
} catch (edm::Exception const&) {
throw;
} catch (cms::Exception& iException) {
//check for "SystemExit: 0" on second line
const std::string& sysexit0("SystemExit: 0");
const auto& msg = iException.message();
size_t pos2 = msg.find('\n');
if (pos2 != std::string::npos and (msg.size() - (pos2 + 1)) > sysexit0.size() and
msg.compare(pos2 + 1, sysexit0.size(), sysexit0) == 0)
return 0;

edm::Exception e(edm::errors::ConfigFileReadError, "", iException);
throw e;
}
Expand All @@ -264,11 +227,11 @@ int main(int argc, char* argv[]) {
auto threadsInfo = threadOptions(*pset);

// check the command line options
if (vm.count(kNumberOfThreadsOpt)) {
threadsInfo.nThreads_ = vm[kNumberOfThreadsOpt].as<unsigned int>();
if (vm.count(edm::CmsRunParser::kNumberOfThreadsOpt)) {
threadsInfo.nThreads_ = vm[edm::CmsRunParser::kNumberOfThreadsOpt].as<unsigned int>();
}
if (vm.count(kSizeOfStackForThreadOpt)) {
threadsInfo.stackSize_ = vm[kSizeOfStackForThreadOpt].as<unsigned int>();
if (vm.count(edm::CmsRunParser::kSizeOfStackForThreadOpt)) {
threadsInfo.stackSize_ = vm[edm::CmsRunParser::kSizeOfStackForThreadOpt].as<unsigned int>();
}

// if needed, re-initialise TBB
Expand All @@ -290,8 +253,8 @@ int main(int argc, char* argv[]) {

context = "Setting MessageLogger defaults";
// Decide what mode of hardcoded MessageLogger defaults to use
if (vm.count(kJobModeOpt)) {
std::string jobMode = vm[kJobModeOpt].as<std::string>();
if (vm.count(edm::CmsRunParser::kJobModeOpt)) {
std::string jobMode = vm[edm::CmsRunParser::kJobModeOpt].as<std::string>();
edm::MessageDrop::instance()->jobMode = jobMode;
}

Expand Down
52 changes: 52 additions & 0 deletions FWCore/Framework/interface/CmsRunParser.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
#ifndef FWCore_Framework_CmsRunParser_h
#define FWCore_Framework_CmsRunParser_h

#include "boost/program_options.hpp"

#include <variant>

namespace edm {
class CmsRunParser {
public:
using MapOrExit = std::variant<boost::program_options::variables_map, int>;
CmsRunParser(const char* name);
MapOrExit parse(int argc, const char* argv[]) const;
//variant helpers
static bool hasVM(const MapOrExit& output) {
return std::holds_alternative<boost::program_options::variables_map>(output);
}
static boost::program_options::variables_map getVM(const MapOrExit& output) {
return std::get<boost::program_options::variables_map>(output);
}
static bool hasExit(const MapOrExit& output) { return std::holds_alternative<int>(output); }
static int getExit(const MapOrExit& output) { return std::get<int>(output); }

private:
boost::program_options::options_description desc_;
boost::program_options::options_description all_options_;
boost::program_options::positional_options_description pos_options_;

public:
//Command line parameters
static inline const char* const kParameterSetOpt = "parameter-set";
static inline const char* const kPythonOpt = "pythonOptions";
static inline const char* const kPythonOptDefault = "CMSRUN_PYTHONOPT_DEFAULT";
static inline const char* const kCmdCommandOpt = "command,c";
static inline const char* const kCmdOpt = "command";
static inline const char* const kJobreportCommandOpt = "jobreport,j";
static inline const char* const kJobreportOpt = "jobreport";
static inline const char* const kEnableJobreportCommandOpt = "enablejobreport,e";
static inline const char* const kEnableJobreportOpt = "enablejobreport";
static inline const char* const kJobModeCommandOpt = "mode,m";
static inline const char* const kJobModeOpt = "mode";
static inline const char* const kNumberOfThreadsCommandOpt = "numThreads,n";
static inline const char* const kNumberOfThreadsOpt = "numThreads";
static inline const char* const kSizeOfStackForThreadCommandOpt = "sizeOfStackForThreadsInKB,s";
static inline const char* const kSizeOfStackForThreadOpt = "sizeOfStackForThreadsInKB";
static inline const char* const kHelpOpt = "help";
static inline const char* const kHelpCommandOpt = "help,h";
static inline const char* const kStrictOpt = "strict";
};
} // namespace edm

#endif
Loading