Accept empty string as unspecified parameter for toml option

ComputationalRadiationPhysics · Nov 25, 2021 · 9508fce · 9508fce
1 parent 18a3510
commit 9508fce
Showing 1 changed file with 38 additions and 57 deletions.
diff --git a/include/picongpu/plugins/openPMD/openPMDWriter.hpp b/include/picongpu/plugins/openPMD/openPMDWriter.hpp
@@ -300,46 +300,11 @@ make sure that environment variable OPENPMD_BP_BACKEND is not set to ADIOS1.
             {
                 if(selfRegister)
                 {
-                    if(notifyPeriod.empty())
-                    {
-                        // In this case, only the --openPMD.toml option is allowed to be defined
-                        if(tomlSources.empty())
-                        {
-                            throw std::runtime_error(name + ": Either parameter period or toml must be defined.");
-                        }
-                        std::vector<plugins::multi::Option<std::string>> theseMustBeEmpty{
-                            source,
-                            fileName,
-                            fileNameExtension,
-                            fileNameInfix,
-                            jsonConfig,
-                            dataPreparationStrategy};
-                        for(auto const& option : theseMustBeEmpty)
-                        {
-                            if(not option.empty())
-                            {
-                                throw std::runtime_error(
-                                    name + ": If using parameter toml, no other parameter may be used (do not define '"
-                                    + option.getName() + "').");
-                            }
-                        }
-                    }
-                    else
-                    {
-                        if(fileName.empty())
-                            throw std::runtime_error(
-                                name + ": if defining parameter period, then parameter file must also be defined");
-                        if(not tomlSources.empty())
-                        {
-                            throw std::runtime_error(
-                                name
-                                + ": If using parameter toml, no other parameter may be used (notifyPeriod was "
-                                  "specified).");
-                        }
-                    }
+                    if(tomlSources.empty() && (notifyPeriod.empty() || fileName.empty()))
+                        throw std::runtime_error(
+                            name + ": If not defining parameter toml, then parameter period and file must be defined");
 
                     // check if user passed data source names are valid
-                    // @todo do this check for TOML-specified sources, too
                     for(auto const& dataSourceNames : source)
                     {
                         auto vectorOfDataSourceNames
@@ -360,19 +325,10 @@ make sure that environment variable OPENPMD_BP_BACKEND is not set to ADIOS1.
             {
                 if(selfRegister)
                 {
-                    if(notifyPeriod.size() == 0)
-                    {
-                        return tomlSources.size();
-                    }
-                    else if(tomlSources.size() == 0)
-                    {
-                        return notifyPeriod.size();
-                    }
-                    else
-                    {
-                        throw std::runtime_error(
-                            "[openPMD plugin] Use either --openPMD.period or --openPMD.toml, not both.");
-                    }
+                    // If using periods in some instances and TOML sources in others, then the other parameter
+                    // must be specified as empty.
+                    // Not this method's task to check this though.
+                    return tomlSources.size() > notifyPeriod.size() ? tomlSources.size() : notifyPeriod.size();
                 }
                 else
                     return 1;
@@ -822,8 +778,31 @@ make sure that environment variable OPENPMD_BP_BACKEND is not set to ADIOS1.
                 {
                     /* only register for notify callback when .period is set on
                      * command line */
-                    if(m_help->tomlSources.optionDefined(m_id) && not m_help->notifyPeriod.optionDefined(m_id))
+                    bool tomlSourcesSpecified
+                        = m_help->tomlSources.optionDefined(m_id) && not m_help->tomlSources.get(m_id).empty();
+                    bool notifyPeriodSpecified
+                        = m_help->notifyPeriod.optionDefined(m_id) && not m_help->notifyPeriod.get(m_id).empty();
+                    if(tomlSourcesSpecified && not notifyPeriodSpecified)
                     {
+                        // Verify that all other parameters are empty for this instance of the plugin
+                        std::vector<plugins::multi::Option<std::string>> theseMustBeEmpty{
+                            m_help->source,
+                            m_help->fileName,
+                            m_help->fileNameExtension,
+                            m_help->fileNameInfix,
+                            m_help->jsonConfig,
+                            m_help->dataPreparationStrategy};
+                        for(auto const& option : theseMustBeEmpty)
+                        {
+                            if(option.optionDefined(m_id) && not option.get(m_id).empty())
+                            {
+                                throw std::runtime_error(
+                                    "[openPMD plugin] If using parameter toml, no other parameter may be used (do not "
+                                    "define '"
+                                    + option.getName() + "').");
+                            }
+                        }
+
                         std::string const& tomlSources = m_help->tomlSources.get(id);
                         mThreadParams.m_configurationSource = ConfigurationVia::Toml;
 
@@ -839,8 +818,12 @@ make sure that environment variable OPENPMD_BP_BACKEND is not set to ADIOS1.
                         /** create notify directory */
                         Environment<simDim>::get().Filesystem().createDirectoryWithPermissions(outputDirectory);
                     }
-                    else if(not m_help->tomlSources.optionDefined(m_id) && m_help->notifyPeriod.optionDefined(m_id))
+                    else if(not tomlSourcesSpecified && notifyPeriodSpecified)
                     {
+                        if(m_help->fileName.empty())
+                            throw std::runtime_error("[openPMD plugin] If defining parameter period, then parameter "
+                                                     "file must also be defined");
+
                         std::string const& notifyPeriod = m_help->notifyPeriod.get(id);
                         mThreadParams.m_configurationSource = ConfigurationVia::CommandLine;
                         Environment<>::get().PluginConnector().setNotificationPeriod(this, notifyPeriod);
@@ -850,10 +833,8 @@ make sure that environment variable OPENPMD_BP_BACKEND is not set to ADIOS1.
                     }
                     else
                     {
-                        throw std::runtime_error(
-                            "Control flow error: Either the notify period or the TOML sources must be "
-                            "specified, but not both. This should have been checked by "
-                            "validateOptions(). If you're seeing this, please report a bug.");
+                        throw std::runtime_error("[openPMD plugin] Either the notify period or the TOML sources must "
+                                                 "be specified, but not both.");
                     }
                 }
             }