From d8294d2a2aad3c737c7b9f07ad9e5b2d826f50b7 Mon Sep 17 00:00:00 2001 From: Ananya Gupta <90386813+Ananya2003Gupta@users.noreply.github.com> Date: Fri, 30 Jun 2023 15:08:16 +0530 Subject: [PATCH 01/22] Make error message from parsing more explicit (#437) --- python/podio/podio_config_reader.py | 32 ++++++++++++++++++----------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/python/podio/podio_config_reader.py b/python/podio/podio_config_reader.py index b6836f0d3..c3d9b4106 100644 --- a/python/podio/podio_config_reader.py +++ b/python/podio/podio_config_reader.py @@ -57,7 +57,7 @@ def _parse_with_regexps(string, regexps_callbacks): if result: return callback(result) - raise DefinitionError(f"'{string}' is not a valid member definition") + raise DefinitionError(f"'{string}' is not a valid member definition. Check syntax of the member definition.") @staticmethod def _full_array_conv(result): @@ -85,18 +85,26 @@ def _bare_member_conv(result): def parse(self, string, require_description=True): """Parse the passed string""" - matchers_cbs = [ + default_matchers_cbs = [ (self.full_array_re, self._full_array_conv), - (self.member_re, self._full_member_conv) - ] - - if not require_description: - matchers_cbs.extend(( - (self.bare_array_re, self._bare_array_conv), - (self.bare_member_re, self._bare_member_conv) - )) - - return self._parse_with_regexps(string, matchers_cbs) + (self.member_re, self._full_member_conv)] + + no_desc_matchers_cbs = [ + (self.bare_array_re, self._bare_array_conv), + (self.bare_member_re, self._bare_member_conv)] + + if require_description: + try: + return self._parse_with_regexps(string, default_matchers_cbs) + except DefinitionError: + # check whether we could parse this if we don't require a description and + # provide more details in the error if we can + self._parse_with_regexps(string, no_desc_matchers_cbs) + # pylint: disable-next=raise-missing-from + raise DefinitionError(f"'{string}' is not a valid member definition. Description comment is missing.\n" + "Correct Syntax: // ") + + return self._parse_with_regexps(string, default_matchers_cbs + no_desc_matchers_cbs) class ClassDefinitionValidator: From 1e8a8f9cf7fcca19da4c508cf339ddbf755057bd Mon Sep 17 00:00:00 2001 From: Juan Miguel Carceller <22276694+jmcarcell@users.noreply.github.com> Date: Tue, 11 Jul 2023 12:18:44 +0200 Subject: [PATCH 02/22] Add support for the new RNTuple format (#395) * Add a RNTuple writer * Cleanup and add a reader * Add compilation instructions for RNTuple * Add tests * Fix the reader and writer so that they pass most of the tests * Commit missing changes in the header * Add support for Generic Parameters * Add an ugly workaround to the unique_ptr issue * Read also vector members and remove some comments * Do a bit of cleanup * Do more cleanup, also compiler warnings * Add names in rootUtils.h, fix a few compiler warnings * Add a few minor changes * Add missing changes in the headers * Change map -> unordered_map and use append in CMakeLists.txt * Simplify writing and reading of generic parameters * Only create the ID table once * Add CollectionInfo structs * Add a ROOT version check * Add missing endif() * Add Name at the end of some names * Add missing Name at the end * Cast to rvalue * Cache entries and reserve * Add comment and remove old comments * Remove a few couts * Remove intermediate variables and use std::move * Run clang-format * Use clang-format on tests too * Enable RNTuple I/O in Key4hep CI * Check if dev3 workflows come with recent enough ROOT * Change MakeField to the new signature * Update the RNTuple reader and writer to use the buffer factory * Run clang-format * Update the RNTuple writer to use a bare model * Add friends for Generic Parameters * Update changes after the changes in the collectionID and string_view * Run clang-format * Update the reader and writer to conform to #405 * Reorganize and clean up code in the reader * Run clang-format * Simplify how the references are filled --------- Co-authored-by: jmcarcell Co-authored-by: tmadlener --- .github/workflows/key4hep.yml | 3 +- .github/workflows/ubuntu.yml | 7 +- CMakeLists.txt | 10 +- include/podio/CollectionBuffers.h | 1 + include/podio/GenericParameters.h | 8 + include/podio/ROOTNTupleReader.h | 104 +++++++++ include/podio/ROOTNTupleWriter.h | 74 ++++++ include/podio/UserDataCollection.h | 2 +- python/templates/CollectionData.cc.jinja2 | 1 + src/CMakeLists.txt | 18 ++ src/ROOTNTupleReader.cc | 186 +++++++++++++++ src/ROOTNTupleWriter.cc | 263 ++++++++++++++++++++++ src/rootUtils.h | 69 ++++++ tests/root_io/CMakeLists.txt | 10 + tests/root_io/read_rntuple.cpp | 6 + tests/root_io/write_rntuple.cpp | 6 + 16 files changed, 762 insertions(+), 6 deletions(-) create mode 100644 include/podio/ROOTNTupleReader.h create mode 100644 include/podio/ROOTNTupleWriter.h create mode 100644 src/ROOTNTupleReader.cc create mode 100644 src/ROOTNTupleWriter.cc create mode 100644 tests/root_io/read_rntuple.cpp create mode 100644 tests/root_io/write_rntuple.cpp diff --git a/.github/workflows/key4hep.yml b/.github/workflows/key4hep.yml index 92240b93e..43f69ef67 100644 --- a/.github/workflows/key4hep.yml +++ b/.github/workflows/key4hep.yml @@ -30,7 +30,8 @@ jobs: -DCMAKE_INSTALL_PREFIX=../install \ -DCMAKE_CXX_STANDARD=17 \ -DCMAKE_CXX_FLAGS=" -fdiagnostics-color=always -Werror -Wno-error=deprecated-declarations " \ - -DUSE_EXTERNAL_CATCH2=AUTO \ + -DUSE_EXTERNAL_CATCH2=ON \ + -DENABLE_RNTUPLE=ON \ -G Ninja .. echo "::endgroup::" echo "::group::Build" diff --git a/.github/workflows/ubuntu.yml b/.github/workflows/ubuntu.yml index 16ae05bee..9814ca7cb 100644 --- a/.github/workflows/ubuntu.yml +++ b/.github/workflows/ubuntu.yml @@ -30,9 +30,10 @@ jobs: -DCMAKE_INSTALL_PREFIX=../install \ -DCMAKE_CXX_STANDARD=17 \ -DCMAKE_CXX_FLAGS=" -fdiagnostics-color=always -Werror -Wno-error=deprecated-declarations " \ - -DUSE_EXTERNAL_CATCH2=OFF \ - -DPODIO_SET_RPATH=ON \ - -G Ninja .. + -DUSE_EXTERNAL_CATCH2=OFF \ + -DPODIO_SET_RPATH=ON \ + -DENABLE_RNTUPLE=ON \ + -G Ninja .. echo "::endgroup::" echo "::group::Build" ninja -k0 diff --git a/CMakeLists.txt b/CMakeLists.txt index de56ea75a..35c521882 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -68,12 +68,20 @@ ADD_CLANG_TIDY() option(CREATE_DOC "Whether or not to create doxygen doc target." OFF) option(ENABLE_SIO "Build SIO I/O support" OFF) option(PODIO_RELAX_PYVER "Do not require exact python version match with ROOT" OFF) +option(ENABLE_RNTUPLE "Build with support for the new ROOT NTtuple format" OFF) #--- Declare ROOT dependency --------------------------------------------------- list(APPEND CMAKE_PREFIX_PATH $ENV{ROOTSYS}) set(CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake) -find_package(ROOT REQUIRED COMPONENTS RIO Tree) +if(NOT ENABLE_RNTUPLE) + find_package(ROOT REQUIRED COMPONENTS RIO Tree) +else() + find_package(ROOT REQUIRED COMPONENTS RIO Tree ROOTNTuple) + if(${ROOT_VERSION} VERSION_LESS 6.28.02) + message(FATAL_ERROR "You are trying to build podio with support for the new ROOT NTuple format, but your ROOT version is too old. Please update ROOT to at least version 6.28.02") + endif() +endif() # Check that root is compiled with a modern enough c++ standard get_target_property(ROOT_COMPILE_FEATURES ROOT::Core INTERFACE_COMPILE_FEATURES) diff --git a/include/podio/CollectionBuffers.h b/include/podio/CollectionBuffers.h index 37ee07fe4..b51161c2d 100644 --- a/include/podio/CollectionBuffers.h +++ b/include/podio/CollectionBuffers.h @@ -27,6 +27,7 @@ using VectorMembersInfo = std::vector>; */ struct CollectionWriteBuffers { void* data{nullptr}; + void* vecPtr{nullptr}; CollRefCollection* references{nullptr}; VectorMembersInfo* vectorMembers{nullptr}; diff --git a/include/podio/GenericParameters.h b/include/podio/GenericParameters.h index 311b622b5..eeee3696f 100644 --- a/include/podio/GenericParameters.h +++ b/include/podio/GenericParameters.h @@ -18,6 +18,11 @@ class write_device; using version_type = uint32_t; // from sio/definitions } // namespace sio +namespace podio { +class ROOTNTupleReader; +class ROOTNTupleWriter; +} // namespace podio + #define DEPR_NON_TEMPLATE \ [[deprecated("Non-templated access will be removed. Switch to templated access functionality")]] @@ -145,6 +150,8 @@ class GenericParameters { friend void writeGenericParameters(sio::write_device& device, const GenericParameters& parameters); friend void readGenericParameters(sio::read_device& device, GenericParameters& parameters, sio::version_type version); + friend ROOTNTupleReader; + friend ROOTNTupleWriter; /// Get a reference to the internal map for a given type template @@ -187,6 +194,7 @@ class GenericParameters { } } +private: /// Get the mutex that guards the map for the given type template std::mutex& getMutex() const { diff --git a/include/podio/ROOTNTupleReader.h b/include/podio/ROOTNTupleReader.h new file mode 100644 index 000000000..a25f66f2a --- /dev/null +++ b/include/podio/ROOTNTupleReader.h @@ -0,0 +1,104 @@ +#ifndef PODIO_ROOTNTUPLEREADER_H +#define PODIO_ROOTNTUPLEREADER_H + +#include "podio/CollectionBranches.h" +#include "podio/ICollectionProvider.h" +#include "podio/ROOTFrameData.h" +#include "podio/SchemaEvolution.h" +#include "podio/podioVersion.h" +#include "podio/utilities/DatamodelRegistryIOHelpers.h" + +#include +#include +#include + +#include +#include + +namespace podio { + +/** +This class has the function to read available data from disk +and to prepare collections and buffers. +**/ +class ROOTNTupleReader { + +public: + ROOTNTupleReader() = default; + ~ROOTNTupleReader() = default; + + ROOTNTupleReader(const ROOTNTupleReader&) = delete; + ROOTNTupleReader& operator=(const ROOTNTupleReader&) = delete; + + void openFile(const std::string& filename); + void openFiles(const std::vector& filename); + + /** + * Read the next data entry from which a Frame can be constructed for the + * given name. In case there are no more entries left for this name or in + * case there is no data for this name, this returns a nullptr. + */ + std::unique_ptr readNextEntry(const std::string& name); + + /** + * Read the specified data entry from which a Frame can be constructed for + * the given name. In case the entry does not exist for this name or in case + * there is no data for this name, this returns a nullptr. + */ + std::unique_ptr readEntry(const std::string& name, const unsigned entry); + + /// Returns number of entries for the given name + unsigned getEntries(const std::string& name); + + /// Get the build version of podio that has been used to write the current file + podio::version::Version currentFileVersion() const { + return m_fileVersion; + } + + void closeFile(); + +private: + /** + * Initialize the given category by filling the maps with metadata information + * that will be used later + */ + bool initCategory(const std::string& category); + + /** + * Read and reconstruct the generic parameters of the Frame + */ + GenericParameters readEventMetaData(const std::string& name, unsigned entNum); + + template + void readParams(const std::string& name, unsigned entNum, GenericParameters& params); + + std::unique_ptr m_metadata{}; + + podio::version::Version m_fileVersion{}; + DatamodelDefinitionHolder m_datamodelHolder{}; + + std::unordered_map>> m_readers{}; + std::unordered_map> m_metadata_readers{}; + std::vector m_filenames{}; + + std::unordered_map m_entries{}; + std::unordered_map m_totalEntries{}; + + struct CollectionInfo { + std::vector id{}; + std::vector name{}; + std::vector type{}; + std::vector isSubsetCollection{}; + std::vector schemaVersion{}; + }; + + std::unordered_map m_collectionInfo{}; + + std::vector m_availableCategories{}; + + std::shared_ptr m_table{}; +}; + +} // namespace podio + +#endif diff --git a/include/podio/ROOTNTupleWriter.h b/include/podio/ROOTNTupleWriter.h new file mode 100644 index 000000000..0f6f6d466 --- /dev/null +++ b/include/podio/ROOTNTupleWriter.h @@ -0,0 +1,74 @@ +#ifndef PODIO_ROOTNTUPLEWRITER_H +#define PODIO_ROOTNTUPLEWRITER_H + +#include "podio/CollectionBase.h" +#include "podio/Frame.h" +#include "podio/GenericParameters.h" +#include "podio/SchemaEvolution.h" +#include "podio/utilities/DatamodelRegistryIOHelpers.h" + +#include "TFile.h" +#include +#include + +#include +#include +#include + +namespace podio { + +class ROOTNTupleWriter { +public: + ROOTNTupleWriter(const std::string& filename); + ~ROOTNTupleWriter(); + + ROOTNTupleWriter(const ROOTNTupleWriter&) = delete; + ROOTNTupleWriter& operator=(const ROOTNTupleWriter&) = delete; + + template + void fillParams(GenericParameters& params, ROOT::Experimental::REntry* entry); + + void writeFrame(const podio::Frame& frame, const std::string& category); + void writeFrame(const podio::Frame& frame, const std::string& category, const std::vector& collsToWrite); + void finish(); + +private: + using StoreCollection = std::pair; + std::unique_ptr createModels(const std::vector& collections); + + std::unique_ptr m_metadata{}; + std::unordered_map> m_writers{}; + std::unique_ptr m_metadataWriter{}; + + std::unique_ptr m_file{}; + + DatamodelDefinitionCollector m_datamodelCollector{}; + + struct CollectionInfo { + std::vector id{}; + std::vector name{}; + std::vector type{}; + std::vector isSubsetCollection{}; + std::vector schemaVersion{}; + }; + + std::unordered_map m_collectionInfo{}; + + std::set m_categories{}; + + bool m_finished{false}; + + std::vector m_intkeys{}, m_floatkeys{}, m_doublekeys{}, m_stringkeys{}; + + std::vector> m_intvalues{}; + std::vector> m_floatvalues{}; + std::vector> m_doublevalues{}; + std::vector> m_stringvalues{}; + + template + std::pair&, std::vector>&> getKeyValueVectors(); +}; + +} // namespace podio + +#endif // PODIO_ROOTNTUPLEWRITER_H diff --git a/include/podio/UserDataCollection.h b/include/podio/UserDataCollection.h index b9aefaf40..cc5b7154f 100644 --- a/include/podio/UserDataCollection.h +++ b/include/podio/UserDataCollection.h @@ -123,7 +123,7 @@ class UserDataCollection : public CollectionBase { /// Get the collection buffers for this collection podio::CollectionWriteBuffers getBuffers() override { _vecPtr = &_vec; // Set the pointer to the correct internal vector - return {&_vecPtr, &m_refCollections, &m_vecmem_info}; + return {&_vecPtr, _vecPtr, &m_refCollections, &m_vecmem_info}; } /// check for validity of the container after read diff --git a/python/templates/CollectionData.cc.jinja2 b/python/templates/CollectionData.cc.jinja2 index 3ae5d3a80..3946ad756 100644 --- a/python/templates/CollectionData.cc.jinja2 +++ b/python/templates/CollectionData.cc.jinja2 @@ -92,6 +92,7 @@ podio::CollectionWriteBuffers {{ class_type }}::getCollectionBuffers(bool isSubs return { isSubsetColl ? nullptr : (void*)&m_data, + isSubsetColl ? nullptr : (void*)m_data.get(), &m_refCollections, // only need to store the ObjectIDs of the referenced objects &m_vecmem_info }; diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 394f23057..daea12e5f 100755 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -34,6 +34,9 @@ FUNCTION(PODIO_ADD_LIB_AND_DICT libname headers sources selection ) $ $) target_link_libraries(${dictname} PUBLIC podio::${libname} podio::podio ROOT::Core ROOT::Tree) + if(ENABLE_RNTUPLE) + target_link_libraries(${dictname} PUBLIC ROOT::ROOTNTuple) + endif() PODIO_GENERATE_DICTIONARY(${dictname} ${headers} SELECTION ${selection} OPTIONS --library ${CMAKE_SHARED_LIBRARY_PREFIX}${dictname}${CMAKE_SHARED_LIBRARY_SUFFIX} ) @@ -83,15 +86,30 @@ SET(root_sources ROOTFrameReader.cc ROOTLegacyReader.cc ) +if(ENABLE_RNTUPLE) + list(APPEND root_sources + ROOTNTupleReader.cc + ROOTNTupleWriter.cc + ) +endif() SET(root_headers ${CMAKE_SOURCE_DIR}/include/podio/ROOTFrameReader.h ${CMAKE_SOURCE_DIR}/include/podio/ROOTLegacyReader.h ${CMAKE_SOURCE_DIR}/include/podio/ROOTFrameWriter.h ) +if(ENABLE_RNTUPLE) + list(APPEND root_headers + ${CMAKE_SOURCE_DIR}/include/podio/ROOTNTupleReader.h + ${CMAKE_SOURCE_DIR}/include/podio/ROOTNTupleWriter.h + ) +endif() PODIO_ADD_LIB_AND_DICT(podioRootIO "${root_headers}" "${root_sources}" root_selection.xml) target_link_libraries(podioRootIO PUBLIC podio::podio ROOT::Core ROOT::RIO ROOT::Tree) +if(ENABLE_RNTUPLE) + target_link_libraries(podioRootIO PUBLIC ROOT::ROOTNTuple) +endif() # --- Python EventStore for enabling (legacy) python bindings diff --git a/src/ROOTNTupleReader.cc b/src/ROOTNTupleReader.cc new file mode 100644 index 000000000..299f88da3 --- /dev/null +++ b/src/ROOTNTupleReader.cc @@ -0,0 +1,186 @@ +#include "podio/ROOTNTupleReader.h" +#include "podio/CollectionBase.h" +#include "podio/CollectionBufferFactory.h" +#include "podio/CollectionBuffers.h" +#include "podio/CollectionIDTable.h" +#include "podio/DatamodelRegistry.h" +#include "podio/GenericParameters.h" +#include "rootUtils.h" + +#include "TClass.h" +#include +#include + +namespace podio { + +template +void ROOTNTupleReader::readParams(const std::string& name, unsigned entNum, GenericParameters& params) { + auto keyView = m_readers[name][0]->GetView>(root_utils::getGPKeyName()); + auto valueView = m_readers[name][0]->GetView>>(root_utils::getGPValueName()); + + for (size_t i = 0; i < keyView(entNum).size(); ++i) { + params.getMap().emplace(std::move(keyView(entNum)[i]), std::move(valueView(entNum)[i])); + } +} + +GenericParameters ROOTNTupleReader::readEventMetaData(const std::string& name, unsigned entNum) { + GenericParameters params; + + readParams(name, entNum, params); + readParams(name, entNum, params); + readParams(name, entNum, params); + readParams(name, entNum, params); + + return params; +} + +bool ROOTNTupleReader::initCategory(const std::string& category) { + if (std::find(m_availableCategories.begin(), m_availableCategories.end(), category) == m_availableCategories.end()) { + return false; + } + // Assume that the metadata is the same in all files + auto filename = m_filenames[0]; + + auto id = m_metadata_readers[filename]->GetView>(root_utils::idTableName(category)); + m_collectionInfo[category].id = id(0); + + auto collectionName = + m_metadata_readers[filename]->GetView>(root_utils::collectionName(category)); + m_collectionInfo[category].name = collectionName(0); + + auto collectionType = + m_metadata_readers[filename]->GetView>(root_utils::collInfoName(category)); + m_collectionInfo[category].type = collectionType(0); + + auto subsetCollection = + m_metadata_readers[filename]->GetView>(root_utils::subsetCollection(category)); + m_collectionInfo[category].isSubsetCollection = subsetCollection(0); + + auto schemaVersion = m_metadata_readers[filename]->GetView>("schemaVersion_" + category); + m_collectionInfo[category].schemaVersion = schemaVersion(0); + + return true; +} + +void ROOTNTupleReader::openFile(const std::string& filename) { + openFiles({filename}); +} + +void ROOTNTupleReader::openFiles(const std::vector& filenames) { + + m_filenames.insert(m_filenames.end(), filenames.begin(), filenames.end()); + for (auto& filename : filenames) { + if (m_metadata_readers.find(filename) == m_metadata_readers.end()) { + m_metadata_readers[filename] = ROOT::Experimental::RNTupleReader::Open(root_utils::metaTreeName, filename); + } + } + + m_metadata = ROOT::Experimental::RNTupleReader::Open(root_utils::metaTreeName, filenames[0]); + + auto versionView = m_metadata->GetView>(root_utils::versionBranchName); + auto version = versionView(0); + + m_fileVersion = podio::version::Version{version[0], version[1], version[2]}; + + auto edmView = m_metadata->GetView>>(root_utils::edmDefBranchName); + auto edm = edmView(0); + + auto availableCategoriesField = m_metadata->GetView>(root_utils::availableCategories); + m_availableCategories = availableCategoriesField(0); +} + +unsigned ROOTNTupleReader::getEntries(const std::string& name) { + if (m_readers.find(name) == m_readers.end()) { + for (auto& filename : m_filenames) { + try { + m_readers[name].emplace_back(ROOT::Experimental::RNTupleReader::Open(name, filename)); + } catch (const ROOT::Experimental::RException& e) { + std::cout << "Category " << name << " not found in file " << filename << std::endl; + } + } + m_totalEntries[name] = std::accumulate(m_readers[name].begin(), m_readers[name].end(), 0, + [](int total, auto& reader) { return total + reader->GetNEntries(); }); + } + return m_totalEntries[name]; +} + +std::unique_ptr ROOTNTupleReader::readNextEntry(const std::string& name) { + return readEntry(name, m_entries[name]); +} + +std::unique_ptr ROOTNTupleReader::readEntry(const std::string& category, const unsigned entNum) { + if (m_totalEntries.find(category) == m_totalEntries.end()) { + getEntries(category); + } + if (entNum >= m_totalEntries[category]) { + return nullptr; + } + + if (m_collectionInfo.find(category) == m_collectionInfo.end()) { + if (!initCategory(category)) { + return nullptr; + } + } + + m_entries[category] = entNum + 1; + + ROOTFrameData::BufferMap buffers; + auto dentry = m_readers[category][0]->GetModel()->GetDefaultEntry(); + + for (size_t i = 0; i < m_collectionInfo[category].id.size(); ++i) { + const auto collectionClass = TClass::GetClass(m_collectionInfo[category].type[i].c_str()); + + auto collection = + std::unique_ptr(static_cast(collectionClass->New())); + + const auto& bufferFactory = podio::CollectionBufferFactory::instance(); + auto maybeBuffers = + bufferFactory.createBuffers(m_collectionInfo[category].type[i], m_collectionInfo[category].schemaVersion[i], + m_collectionInfo[category].isSubsetCollection[i]); + auto collBuffers = maybeBuffers.value_or(podio::CollectionReadBuffers{}); + + if (!maybeBuffers) { + std::cout << "WARNING: Buffers couldn't be created for collection " << m_collectionInfo[category].name[i] + << " of type " << m_collectionInfo[category].type[i] << " and schema version " + << m_collectionInfo[category].schemaVersion[i] << std::endl; + return nullptr; + } + + if (m_collectionInfo[category].isSubsetCollection[i]) { + auto brName = root_utils::subsetBranch(m_collectionInfo[category].name[i]); + auto vec = new std::vector; + dentry->CaptureValueUnsafe(brName, vec); + collBuffers.references->at(0) = std::unique_ptr>(vec); + } else { + dentry->CaptureValueUnsafe(m_collectionInfo[category].name[i], collBuffers.data); + + const auto relVecNames = podio::DatamodelRegistry::instance().getRelationNames(collection->getTypeName()); + for (size_t j = 0; j < relVecNames.relations.size(); ++j) { + const auto relName = relVecNames.relations[j]; + auto vec = new std::vector; + const auto brName = root_utils::refBranch(m_collectionInfo[category].name[i], relName); + dentry->CaptureValueUnsafe(brName, vec); + collBuffers.references->at(j) = std::unique_ptr>(vec); + } + + for (size_t j = 0; j < relVecNames.vectorMembers.size(); ++j) { + const auto vecName = relVecNames.vectorMembers[j]; + const auto brName = root_utils::vecBranch(m_collectionInfo[category].name[i], vecName); + dentry->CaptureValueUnsafe(brName, collBuffers.vectorMembers->at(j).second); + } + } + + buffers.emplace(m_collectionInfo[category].name[i], std::move(collBuffers)); + } + + m_readers[category][0]->LoadEntry(entNum); + + auto parameters = readEventMetaData(category, entNum); + if (!m_table) { + m_table = std::make_shared(m_collectionInfo[category].id, m_collectionInfo[category].name); + } + + return std::make_unique(std::move(buffers), m_table, std::move(parameters)); +} + +} // namespace podio diff --git a/src/ROOTNTupleWriter.cc b/src/ROOTNTupleWriter.cc new file mode 100644 index 000000000..741af53b3 --- /dev/null +++ b/src/ROOTNTupleWriter.cc @@ -0,0 +1,263 @@ +#include "podio/ROOTNTupleWriter.h" +#include "podio/CollectionBase.h" +#include "podio/DatamodelRegistry.h" +#include "podio/GenericParameters.h" +#include "podio/SchemaEvolution.h" +#include "podio/podioVersion.h" +#include "rootUtils.h" + +#include "TFile.h" +#include +#include +#include + +#include + +namespace podio { + +ROOTNTupleWriter::ROOTNTupleWriter(const std::string& filename) : + m_metadata(ROOT::Experimental::RNTupleModel::Create()), + m_file(new TFile(filename.c_str(), "RECREATE", "data file")) { +} + +ROOTNTupleWriter::~ROOTNTupleWriter() { + if (!m_finished) { + finish(); + } +} + +template +std::pair&, std::vector>&> ROOTNTupleWriter::getKeyValueVectors() { + if constexpr (std::is_same_v) { + return {m_intkeys, m_intvalues}; + } else if constexpr (std::is_same_v) { + return {m_floatkeys, m_floatvalues}; + } else if constexpr (std::is_same_v) { + return {m_doublekeys, m_doublevalues}; + } else if constexpr (std::is_same_v) { + return {m_stringkeys, m_stringvalues}; + } else { + throw std::runtime_error("Unknown type"); + } +} + +template +void ROOTNTupleWriter::fillParams(GenericParameters& params, ROOT::Experimental::REntry* entry) { + auto [key, value] = getKeyValueVectors(); + entry->CaptureValueUnsafe(root_utils::getGPKeyName(), &key); + entry->CaptureValueUnsafe(root_utils::getGPValueName(), &value); + + key.clear(); + key.reserve(params.getMap().size()); + value.clear(); + value.reserve(params.getMap().size()); + + for (auto& [kk, vv] : params.getMap()) { + key.emplace_back(kk); + value.emplace_back(vv); + } +} + +void ROOTNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& category) { + writeFrame(frame, category, frame.getAvailableCollections()); +} + +void ROOTNTupleWriter::writeFrame(const podio::Frame& frame, const std::string& category, + const std::vector& collsToWrite) { + + std::vector collections; + collections.reserve(collsToWrite.size()); + for (const auto& name : collsToWrite) { + auto* coll = frame.getCollectionForWrite(name); + collections.emplace_back(name, const_cast(coll)); + } + + bool new_category = false; + if (m_writers.find(category) == m_writers.end()) { + new_category = true; + auto model = createModels(collections); + m_writers[category] = ROOT::Experimental::RNTupleWriter::Append(std::move(model), category, *m_file.get(), {}); + } + + auto entry = m_writers[category]->GetModel()->CreateBareEntry(); + + ROOT::Experimental::RNTupleWriteOptions options; + options.SetCompression(ROOT::RCompressionSetting::EDefaults::kUseGeneralPurpose); + + for (const auto& [name, coll] : collections) { + auto collBuffers = coll->getBuffers(); + if (collBuffers.vecPtr) { + entry->CaptureValueUnsafe(name, (void*)collBuffers.vecPtr); + } + + if (coll->isSubsetCollection()) { + auto& refColl = (*collBuffers.references)[0]; + const auto brName = root_utils::subsetBranch(name); + entry->CaptureValueUnsafe(brName, refColl.get()); + } else { + + const auto relVecNames = podio::DatamodelRegistry::instance().getRelationNames(coll->getValueTypeName()); + if (auto refColls = collBuffers.references) { + int i = 0; + for (auto& c : (*refColls)) { + const auto brName = root_utils::refBranch(name, relVecNames.relations[i]); + entry->CaptureValueUnsafe(brName, c.get()); + ++i; + } + } + + if (auto vmInfo = collBuffers.vectorMembers) { + int i = 0; + for (auto& [type, vec] : (*vmInfo)) { + const auto typeName = "vector<" + type + ">"; + const auto brName = root_utils::vecBranch(name, relVecNames.vectorMembers[i]); + auto ptr = *(std::vector**)vec; + entry->CaptureValueUnsafe(brName, ptr); + ++i; + } + } + } + + // Not supported + // entry->CaptureValueUnsafe(root_utils::paramBranchName, + // &const_cast(frame.getParameters())); + + if (new_category) { + m_collectionInfo[category].id.emplace_back(coll->getID()); + m_collectionInfo[category].name.emplace_back(name); + m_collectionInfo[category].type.emplace_back(coll->getTypeName()); + m_collectionInfo[category].isSubsetCollection.emplace_back(coll->isSubsetCollection()); + m_collectionInfo[category].schemaVersion.emplace_back(coll->getSchemaVersion()); + } + } + + auto params = frame.getParameters(); + fillParams(params, entry.get()); + fillParams(params, entry.get()); + fillParams(params, entry.get()); + fillParams(params, entry.get()); + + m_writers[category]->Fill(*entry); + m_categories.insert(category); +} + +std::unique_ptr +ROOTNTupleWriter::createModels(const std::vector& collections) { + auto model = ROOT::Experimental::RNTupleModel::CreateBare(); + for (auto& [name, coll] : collections) { + const auto collBuffers = coll->getBuffers(); + + if (collBuffers.vecPtr) { + auto collClassName = "std::vector<" + std::string(coll->getDataTypeName()) + ">"; + auto field = ROOT::Experimental::Detail::RFieldBase::Create(name, collClassName).Unwrap(); + model->AddField(std::move(field)); + } + + if (coll->isSubsetCollection()) { + const auto brName = root_utils::subsetBranch(name); + auto collClassName = "vector"; + auto field = ROOT::Experimental::Detail::RFieldBase::Create(brName, collClassName).Unwrap(); + model->AddField(std::move(field)); + } else { + + const auto relVecNames = podio::DatamodelRegistry::instance().getRelationNames(coll->getValueTypeName()); + if (auto refColls = collBuffers.references) { + int i = 0; + for (auto& c [[maybe_unused]] : (*refColls)) { + const auto brName = root_utils::refBranch(name, relVecNames.relations[i]); + auto collClassName = "vector"; + auto field = ROOT::Experimental::Detail::RFieldBase::Create(brName, collClassName).Unwrap(); + model->AddField(std::move(field)); + ++i; + } + } + + if (auto vminfo = collBuffers.vectorMembers) { + int i = 0; + for (auto& [type, vec] : (*vminfo)) { + const auto typeName = "vector<" + type + ">"; + const auto brName = root_utils::vecBranch(name, relVecNames.vectorMembers[i]); + auto field = ROOT::Experimental::Detail::RFieldBase::Create(brName, typeName).Unwrap(); + model->AddField(std::move(field)); + ++i; + } + } + } + } + + // Not supported by ROOT because podio::GenericParameters has map types + // so we have to split them manually + // model->MakeField(root_utils::paramBranchName); + + model->AddField( + ROOT::Experimental::Detail::RFieldBase::Create(root_utils::intKeyName, "std::vector>").Unwrap()); + model->AddField( + ROOT::Experimental::Detail::RFieldBase::Create(root_utils::floatKeyName, "std::vector>").Unwrap()); + model->AddField( + ROOT::Experimental::Detail::RFieldBase::Create(root_utils::doubleKeyName, "std::vector>").Unwrap()); + model->AddField( + ROOT::Experimental::Detail::RFieldBase::Create(root_utils::stringKeyName, "std::vector>").Unwrap()); + + model->AddField( + ROOT::Experimental::Detail::RFieldBase::Create(root_utils::intValueName, "std::vector>") + .Unwrap()); + model->AddField( + ROOT::Experimental::Detail::RFieldBase::Create(root_utils::floatValueName, "std::vector>") + .Unwrap()); + model->AddField( + ROOT::Experimental::Detail::RFieldBase::Create(root_utils::doubleValueName, "std::vector>") + .Unwrap()); + model->AddField(ROOT::Experimental::Detail::RFieldBase::Create(root_utils::stringValueName, + "std::vector>") + .Unwrap()); + + model->Freeze(); + return model; +} + +void ROOTNTupleWriter::finish() { + + auto podioVersion = podio::version::build_version; + auto versionField = m_metadata->MakeField>(root_utils::versionBranchName); + *versionField = {podioVersion.major, podioVersion.minor, podioVersion.patch}; + + auto edmDefinitions = m_datamodelCollector.getDatamodelDefinitionsToWrite(); + auto edmField = + m_metadata->MakeField>>(root_utils::edmDefBranchName); + *edmField = edmDefinitions; + + auto availableCategoriesField = m_metadata->MakeField>(root_utils::availableCategories); + for (auto& [c, _] : m_collectionInfo) { + availableCategoriesField->push_back(c); + } + + for (auto& category : m_categories) { + auto idField = m_metadata->MakeField>({root_utils::idTableName(category)}); + *idField = m_collectionInfo[category].id; + auto collectionNameField = m_metadata->MakeField>({root_utils::collectionName(category)}); + *collectionNameField = m_collectionInfo[category].name; + auto collectionTypeField = m_metadata->MakeField>({root_utils::collInfoName(category)}); + *collectionTypeField = m_collectionInfo[category].type; + auto subsetCollectionField = m_metadata->MakeField>({root_utils::subsetCollection(category)}); + *subsetCollectionField = m_collectionInfo[category].isSubsetCollection; + auto schemaVersionField = m_metadata->MakeField>({"schemaVersion_" + category}); + *schemaVersionField = m_collectionInfo[category].schemaVersion; + } + + m_metadata->Freeze(); + m_metadataWriter = + ROOT::Experimental::RNTupleWriter::Append(std::move(m_metadata), root_utils::metaTreeName, *m_file, {}); + + m_metadataWriter->Fill(); + + m_file->Write(); + + // All the tuple writers must be deleted before the file so that they flush + // unwritten output + m_writers.clear(); + m_metadataWriter.reset(); + + m_finished = true; +} + +} // namespace podio diff --git a/src/rootUtils.h b/src/rootUtils.h index 507d24b15..523d5c228 100644 --- a/src/rootUtils.h +++ b/src/rootUtils.h @@ -32,6 +32,75 @@ constexpr static auto metaTreeName = "podio_metadata"; */ constexpr static auto paramBranchName = "PARAMETERS"; +/** + * Names of the fields with the keys and values of the generic parameters for + * the RNTuples until map types are supported + */ +constexpr static auto intKeyName = "GPIntKeys"; +constexpr static auto floatKeyName = "GPFloatKeys"; +constexpr static auto doubleKeyName = "GPDoubleKeys"; +constexpr static auto stringKeyName = "GPStringKeys"; + +constexpr static auto intValueName = "GPIntValues"; +constexpr static auto floatValueName = "GPFloatValues"; +constexpr static auto doubleValueName = "GPDoubleValues"; +constexpr static auto stringValueName = "GPStringValues"; + +/** + * Get the name of the key depending on the type + */ +template +constexpr auto getGPKeyName() { + if constexpr (std::is_same::value) { + return intKeyName; + } else if constexpr (std::is_same::value) { + return floatKeyName; + } else if constexpr (std::is_same::value) { + return doubleKeyName; + } else if constexpr (std::is_same::value) { + return stringKeyName; + } else { + static_assert(sizeof(T) == 0, "Unsupported type for generic parameters"); + } +} + +/** + * Get the name of the value depending on the type + */ +template +constexpr auto getGPValueName() { + if constexpr (std::is_same::value) { + return intValueName; + } else if constexpr (std::is_same::value) { + return floatValueName; + } else if constexpr (std::is_same::value) { + return doubleValueName; + } else if constexpr (std::is_same::value) { + return stringValueName; + } else { + static_assert(sizeof(T) == 0, "Unsupported type for generic parameters"); + } +} + +/** + * Name of the field with the list of categories for RNTuples + */ +constexpr static auto availableCategories = "availableCategories"; + +/** + * Name of the field with the names of the collections for RNTuples + */ +inline std::string collectionName(const std::string& category) { + return category + "_collectionNames"; +} + +/** + * Name of the field with the flag for subset collections for RNTuples + */ +inline std::string subsetCollection(const std::string& category) { + return category + "_isSubsetCollections"; +} + /** * The name of the branch into which we store the build version of podio at the * time of writing the file diff --git a/tests/root_io/CMakeLists.txt b/tests/root_io/CMakeLists.txt index d0d8b21c4..ad1537c23 100644 --- a/tests/root_io/CMakeLists.txt +++ b/tests/root_io/CMakeLists.txt @@ -12,6 +12,13 @@ set(root_dependent_tests read_frame_legacy_root.cpp read_frame_root_multiple.cpp ) +if(ENABLE_RNTUPLE) + set(root_dependent_tests + ${root_dependent_tests} + write_rntuple.cpp + read_rntuple.cpp + ) +endif() set(root_libs TestDataModelDict ExtensionDataModelDict podio::podioRootIO) foreach( sourcefile ${root_dependent_tests} ) CREATE_PODIO_TEST(${sourcefile} "${root_libs}") @@ -27,6 +34,9 @@ set_property(TEST read_frame_legacy_root PROPERTY DEPENDS write) set_property(TEST read_timed PROPERTY DEPENDS write_timed) set_property(TEST read_frame_root PROPERTY DEPENDS write_frame_root) set_property(TEST read_frame_root_multiple PROPERTY DEPENDS write_frame_root) +if(ENABLE_RNTUPLE) + set_property(TEST read_rntuple PROPERTY DEPENDS write_rntuple) +endif() add_test(NAME check_benchmark_outputs COMMAND check_benchmark_outputs write_benchmark_root.root read_benchmark_root.root) set_property(TEST check_benchmark_outputs PROPERTY DEPENDS read_timed write_timed) diff --git a/tests/root_io/read_rntuple.cpp b/tests/root_io/read_rntuple.cpp new file mode 100644 index 000000000..59688b2f2 --- /dev/null +++ b/tests/root_io/read_rntuple.cpp @@ -0,0 +1,6 @@ +#include "podio/ROOTNTupleReader.h" +#include "read_frame.h" + +int main() { + return read_frames("example_rntuple.root"); +} diff --git a/tests/root_io/write_rntuple.cpp b/tests/root_io/write_rntuple.cpp new file mode 100644 index 000000000..ce7810c53 --- /dev/null +++ b/tests/root_io/write_rntuple.cpp @@ -0,0 +1,6 @@ +#include "podio/ROOTNTupleWriter.h" +#include "write_frame.h" + +int main() { + write_frames("example_rntuple.root"); +} From 9ac7d32968d0b46da208982a478dd52e6de1b17a Mon Sep 17 00:00:00 2001 From: Juan Miguel Carceller <22276694+jmcarcell@users.noreply.github.com> Date: Tue, 11 Jul 2023 12:27:12 +0200 Subject: [PATCH 03/22] Allow the writers not to call finish() (#442) Co-authored-by: jmcarcell --- include/podio/ROOTFrameWriter.h | 4 +++- include/podio/SIOWriter.h | 4 +++- src/ROOTFrameWriter.cc | 8 ++++++++ src/SIOWriter.cc | 8 ++++++++ 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/include/podio/ROOTFrameWriter.h b/include/podio/ROOTFrameWriter.h index 535b84025..addc22692 100644 --- a/include/podio/ROOTFrameWriter.h +++ b/include/podio/ROOTFrameWriter.h @@ -24,7 +24,7 @@ class GenericParameters; class ROOTFrameWriter { public: ROOTFrameWriter(const std::string& filename); - ~ROOTFrameWriter() = default; + ~ROOTFrameWriter(); ROOTFrameWriter(const ROOTFrameWriter&) = delete; ROOTFrameWriter& operator=(const ROOTFrameWriter&) = delete; @@ -83,6 +83,8 @@ class ROOTFrameWriter { std::unordered_map m_categories{}; ///< All categories DatamodelDefinitionCollector m_datamodelCollector{}; + + bool m_finished{false}; ///< Whether writing has been actually done }; } // namespace podio diff --git a/include/podio/SIOWriter.h b/include/podio/SIOWriter.h index 29ce0bc7d..7594ae74a 100644 --- a/include/podio/SIOWriter.h +++ b/include/podio/SIOWriter.h @@ -20,7 +20,7 @@ class DEPR_EVTSTORE SIOWriter { public: SIOWriter(const std::string& filename, EventStore* store); - ~SIOWriter() = default; + ~SIOWriter(); // non-copyable SIOWriter(const SIOWriter&) = delete; @@ -46,6 +46,8 @@ class DEPR_EVTSTORE SIOWriter { std::shared_ptr m_collectionMetaData; SIOFileTOCRecord m_tocRecord{}; std::vector m_collectionsToWrite{}; + + bool m_finished{false}; }; } // namespace podio diff --git a/src/ROOTFrameWriter.cc b/src/ROOTFrameWriter.cc index e24523da7..af7126596 100644 --- a/src/ROOTFrameWriter.cc +++ b/src/ROOTFrameWriter.cc @@ -15,6 +15,12 @@ ROOTFrameWriter::ROOTFrameWriter(const std::string& filename) { m_file = std::make_unique(filename.c_str(), "recreate"); } +ROOTFrameWriter::~ROOTFrameWriter() { + if (!m_finished) { + finish(); + } +} + void ROOTFrameWriter::writeFrame(const podio::Frame& frame, const std::string& category) { writeFrame(frame, category, frame.getAvailableCollections()); } @@ -143,6 +149,8 @@ void ROOTFrameWriter::finish() { m_file->Write(); m_file->Close(); + + m_finished = true; } } // namespace podio diff --git a/src/SIOWriter.cc b/src/SIOWriter.cc index 7cff1fa98..1c1221c13 100644 --- a/src/SIOWriter.cc +++ b/src/SIOWriter.cc @@ -34,6 +34,12 @@ SIOWriter::SIOWriter(const std::string& filename, EventStore* store) : auto& libLoader [[maybe_unused]] = SIOBlockLibraryLoader::instance(); } +SIOWriter::~SIOWriter() { + if (!m_finished) { + finish(); + } +} + void SIOWriter::writeEvent() { if (m_firstEvent) { // Write the collectionIDs as a separate record at the beginning of the @@ -86,6 +92,8 @@ void SIOWriter::finish() { m_stream.write(reinterpret_cast(&finalWords), sizeof(finalWords)); m_stream.close(); + + m_finished = true; } void SIOWriter::registerForWrite(const std::string& name) { From 6409d5835b26bc81904b84449ce5d02f5a75d7d3 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Thu, 13 Jul 2023 09:36:03 +0200 Subject: [PATCH 04/22] Make the SIOFrameWriter call to finish non-mandatory (#446) --- include/podio/SIOFrameWriter.h | 3 ++- src/SIOFrameWriter.cc | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/include/podio/SIOFrameWriter.h b/include/podio/SIOFrameWriter.h index a8a7d084f..c111261ca 100644 --- a/include/podio/SIOFrameWriter.h +++ b/include/podio/SIOFrameWriter.h @@ -17,7 +17,7 @@ class Frame; class SIOFrameWriter { public: SIOFrameWriter(const std::string& filename); - ~SIOFrameWriter() = default; + ~SIOFrameWriter(); SIOFrameWriter(const SIOFrameWriter&) = delete; SIOFrameWriter& operator=(const SIOFrameWriter&) = delete; @@ -37,6 +37,7 @@ class SIOFrameWriter { sio::ofstream m_stream{}; ///< The output file stream SIOFileTOCRecord m_tocRecord{}; ///< The "table of contents" of the written file DatamodelDefinitionCollector m_datamodelCollector{}; + bool m_finished{false}; ///< Has finish been called already? }; } // namespace podio diff --git a/src/SIOFrameWriter.cc b/src/SIOFrameWriter.cc index 360c948d2..85f497afe 100644 --- a/src/SIOFrameWriter.cc +++ b/src/SIOFrameWriter.cc @@ -26,6 +26,12 @@ SIOFrameWriter::SIOFrameWriter(const std::string& filename) { sio_utils::writeRecord(blocks, "podio_header_info", m_stream, sizeof(podio::version::Version), false); } +SIOFrameWriter::~SIOFrameWriter() { + if (!m_finished) { + finish(); + } +} + void SIOFrameWriter::writeFrame(const podio::Frame& frame, const std::string& category) { writeFrame(frame, category, frame.getAvailableCollections()); } From 654ad98b9997aa6c5a7598ea5e661c19f58a0461 Mon Sep 17 00:00:00 2001 From: tmadlener Date: Thu, 13 Jul 2023 09:52:34 +0200 Subject: [PATCH 05/22] Make sure to only run finish once --- src/SIOFrameWriter.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/SIOFrameWriter.cc b/src/SIOFrameWriter.cc index 85f497afe..dcc1e4c22 100644 --- a/src/SIOFrameWriter.cc +++ b/src/SIOFrameWriter.cc @@ -76,6 +76,8 @@ void SIOFrameWriter::finish() { m_stream.write(reinterpret_cast(&finalWords), sizeof(finalWords)); m_stream.close(); + + m_finished = true; } } // namespace podio From c13ada895df12044ec6a07fcc8410142b2b42928 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Thu, 13 Jul 2023 13:06:23 +0200 Subject: [PATCH 06/22] Remove the lcio datalayout (#448) --- lcio/datalayout.yaml | 224 ------------------------------------------- 1 file changed, 224 deletions(-) delete mode 100644 lcio/datalayout.yaml diff --git a/lcio/datalayout.yaml b/lcio/datalayout.yaml deleted file mode 100644 index a031e3c11..000000000 --- a/lcio/datalayout.yaml +++ /dev/null @@ -1,224 +0,0 @@ ---- - -# LCIO test description -# Changes w.r.t. to original: -# o no collection specific information implemented for now -# o no string member in Vertex - -datatypes : - - # LCIO RawCalorimeterHit - RawCalorimeterHit: - description: "LCIO raw calorimeter hit" - author : "F.Gaede, B. Hegner" - members: - - int cellID0 // The detector specific (geometrical) cell id. - - int cellID1 // The second detector specific (geometrical) cell id. - - int amplitude // The amplitude of the hit in ADC counts. - - int timeStamp // The time stamp for the hit. - - # LCIO CalorimeterHit - CalorimeterHit: - description: "LCIO calorimeter hit" - author : "F.Gaede, B. Hegner" - members: - - int cellID0 // The detector specific (geometrical) cell id. - - int cellID1 // The second detector specific (geometrical) cell id. - - float energy // The energy of the hit in [GeV]. - - float time // The time of the hit in [ns]. - - float[3] position // The position of the hit in world coordinates. - - int type // The type of the hit - OneToOneRelations: - - RawCalorimeterHit rawHit // The RawCalorimeterHit - - # LCIO cluster - # Changes w.r.t. to original: - # o std::bitset< 32 > _type not yet mapped - # o ParticleIDs are not implemented - # o cluster weight not properly implemented - Cluster: - description: "LCIO cluster" - author : "F.Gaede, B. Hegner" - members: - - float energy // Energy of the cluster - - float[3] position // Position of the cluster. - - float[6] positionError // Covariance matrix of the position (6 Parameters) - - float theta // Intrinsic direction of cluster at position - Theta. - - float phi // Intrinsic direction of cluster at position - Phi. - - float[3] directionError // Covariance matrix of the direction (3 Parameters) - - std::vector shape // Shape parameters - - std::vector weight // weight of a particular cluster - - std::vector subdetectorEnergies // A vector that holds the energy observed in a particular subdetector. - OneToManyRelations: - - Cluster clusters // The clusters that have been combined to this cluster. - - CalorimeterHit hits // The hits that have been combined to this cluster. - - # LCIO MCParticle - # Changes w.r.t. to original: - # o std::bitset< 32 > _simstatus not yet implemented - MCParticle: - description: "LCIO MC Particle" - author : "F.Gaede, B. Hegner" - members: - - int pdg // The PDG code of the particle. - - int genstatus // The status for particles as defined by the generator. - - float[3] vertex // The production vertex of the particle in [mm]. - - float charge // The particle's charge. - - float mass // The mass of the particle in [GeV] - - float time // The creation time of the particle in [ns] wrt. the event, e.g. for preassigned decays or decays in flight from the simulator. - - float[3] endpoint // The endpoint of the particle in [mm] - - bool endpointSet // Whether the endpoint has been set - - float[3] momentum // The particle's 3-momentum at the production vertex in [GeV] - OneToManyRelations: - - MCParticle parents // The parents of this particle. - - MCParticle daughters // The daughters this particle. - - # LCIO ReconstructedParticle - ReconstructedParticle: - description: "LCIO Reconstructed Particle" - author : "F.Gaede, B. Hegner" - members: - - int type // Type of reconstructed particle. - - float energy // Energy of the reconstructed particle. - - float[3] momentum // The reconstructed particle's 3-momentum - - float charge // The particle's charge - - float mass // The mass of the particle in [GeV] - OneToOneRelations: - - Vertex vertex // The start vertex associated to this particle. - OneToManyRelations: - - Cluster clusters // The clusters combined to this particle. - - Track tracks // The tracks combined to this particle" - - ReconstructedParticle particles // The particles combined to this particle - -#EVENT::FloatVec _cov -#float _reference [3] -#EVENT::ParticleID * _pidUsed -#float _goodnessOfPID -#EVENT::ParticleIDVec _pid - - # LCIO SimCalorimeterHit - # Changes w.r.t. to original: - # o MCParticleContribution has to become its own collection - SimCalorimeterHit: - description: "LCIO simulated calorimeter hit" - author : "F.Gaede, B. Hegner" - members: - - int cellID0 // The detector specific (geometrical) cell id. - - int cellID1 // The second detector specific (geometrical) cell id. - - float energy // The energy of the hit in [GeV]. - - float time // The time of the hit in [ns]. - - float[3] position // The position of the hit in world coordinates. - - # LCIO SimTrackerHit - SimTrackerHit: - description: "LCIO simulated tracker hit" - author : "F.Gaede, B. Hegner" - members: - - int cellID0 // The detector specific (geometrical) cell id. - - int cellID1 // The second detector specific (geometrical) cell id. - - float[3] position // The position of the hit in world coordinates. - - float time // The time of the hit in [ns]. - - float pathLength // path length - - float EDep // EDep - - float _p [3] // position? - OneToOneRelations: - - MCParticle particle // The MCParticle that caused the hit. - - - # LCIO Track - # o not yet implemented std::bitset< 32 > _type - Track: - description: "LCIO reconstructed track" - author : "F.Gaede, B. Hegner" - members: - - float chi2 // Chi2 - - int ndf // Number of degrees of freedom of the track fit. - - float dEdx // dEdx of the track. - - float dEdxError // Error of dEdx. - - float radiusOfInnermostHit // The radius of the innermost hit that has been used in the track fit. - - std::vector subdetectorHitNumbers // The number of hits in particular subdetectors - OnoToManyRelations: - - Track tracks // The tracks that have been combined to this track. - - TrackerHit hits // The hits that have been combined to this track. - - TrackState trackStates // Track states associated to this track. - - # LCIO TrackerData - TrackerData: - description: "LCIO tracker data" - author : "F.Gaede, B. Hegner" - members: - - int cellID0 // The detector specific (geometrical) cell id. - - int cellID1 // The second detector specific (geometrical) cell id. - - int time // The time of the hit. - - std::vector charge // The corrected (calibrated) FADC spectrum. - - # LCIO TrackerHit - # o no specialisation for the different kind of geometries - TrackerHit: - description : "LCIO tracker hit" - author : "F.Gaede, B. Hegner" - members : - - int cellID0 // The detector specific (geometrical) cell id. - - int cellID1 // The second detector specific (geometrical) cell id. - - int time // The time of the hit. - - float EDep // EDep - - float EDepError // error on EDep - - float[3] position // ... - - std::vector // The calibrated ADC values - -#int _type -#EVENT::FloatVec _cov -#int _quality -#EVENT::LCObjectVec _rawHits - - # LCIO TrackerPulse - TrackerPulse: - description : "LCIO tracker pulse" - author : "F. Gaede, B. Hegner" - members: - - int cellID0 // The detector specific (geometrical) cell id. - - int cellID1 // The second detector specific (geometrical) cell id. - - int quality // ... - - float time // The time of the pulse. - - float charge // The integrated charge of the pulse - - std::vector cov // ... - OneToOneRelations: - - TrackerData corrData // ... - - # LCIO TrackerRawData - TrackerData: - description: "LCIO tracker raw data" - author : "F.Gaede, B. Hegner" - members: - - int cellID0 // The detector specific (geometrical) cell id. - - int cellID1 // The second detector specific (geometrical) cell id. - - int time // time measurement associated with the adc values. - - std::vector charge // The actual FADC spectrum. - - # LCIO TrackState - TrackState: - description: "LCIO track state" - author : "F.Gaede, B. Hegner" - members: - - int location // The location of the track state. - - float d0 // Impact parameter of the track in (r-phi). - - float phi // Phi of the track at the reference point. - - float omega // Omega is the signed curvature of the track in [1/mm]. - - float z0 // Impact parameter of the track in (r-z). - - float tanLambda // Lambda is the dip angle of the track in r-z at the reference point. - - float[3] referencePoint // Reference point of the track parameters - - std::vector covMatrix // Covariance matrix of the track parameters. - - # LCIO Vertex - Vertex: - description: "LCIO vertex" - author : "F.Gaede, B. Hegner" - members: - - int primary // Whether it is the primary vertex of the event - - float chi2 // Chi squared of the vertex fit. - - float probability // Probability of the vertex fit - - float[3] position // Position of the vertex. - - std::vector cov // - - std::vector par // - OneToOneRelations: - - ReconstructedParticle particle // Reconstructed Particle associated to the Vertex. From eb9276bcb5d3a315ffab4d5296b499a8bc16743e Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Thu, 13 Jul 2023 15:47:28 +0200 Subject: [PATCH 07/22] Fix the pre-commit workflow (#449) * Switch to a newer LCG stack with root 6.28/00 * Try with key4hep nightlies * Allow git to run necessary commands for pre-commit * Fix clang-format complaints * Fix clang-tidy complaints * Reorder some imports to make pylint happy again * Enbale building RNTuple support in more workflows --- .github/workflows/pre-commit.yml | 13 +++++++++---- .github/workflows/test.yml | 14 +++++++++----- python/podio/root_io.py | 4 ++-- python/podio/sio_io.py | 4 ++-- python/podio_class_generator.py | 2 +- src/EventStore.cc | 2 +- src/ROOTReader.cc | 2 +- src/SIOBlockUserData.cc | 21 --------------------- src/SchemaEvolution.cc | 2 +- 9 files changed, 26 insertions(+), 38 deletions(-) diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index cc4eeba90..70d60a73b 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -15,19 +15,24 @@ jobs: - uses: cvmfs-contrib/github-action-cvmfs@v3 - uses: aidasoft/run-lcg-view@v4 with: - release-platform: LCG_102/x86_64-centos7-clang12-opt + container: centos7 + view-path: /cvmfs/sw-nightlies.hsf.org/key4hep run: | echo "::group::Setup pre-commit" - export PYTHONPATH=$(python -m site --user-site):$PYTHONPATH export PATH=/root/.local/bin:$PATH + # Newer versions of git are more cautious around the github runner + # environment and without this git rev-parse --show-cdup in pre-commit + # fails + git config --global --add safe.directory $(pwd) pip install pre-commit - # Use virtualenv from the LCG release - pip uninstall --yes virtualenv + pip install pylint==2.12.2 + pip install flake8 echo "::endgroup::" echo "::group::Run CMake" mkdir build cd build cmake .. -DENABLE_SIO=ON \ + -DENABLE_RNTUPLE=ON \ -DCMAKE_CXX_STANDARD=17 \ -DCMAKE_CXX_FLAGS=" -fdiagnostics-color=always -Werror "\ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a57fa321a..858d20849 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -13,12 +13,15 @@ jobs: strategy: fail-fast: false matrix: - sio: [ON] - LCG: ["LCG_102/x86_64-centos7-clang12-opt", - "LCG_102/x86_64-centos8-gcc11-opt", - "dev3/x86_64-centos7-clang12-opt", + RNTUPLE: [ON] + LCG: ["dev3/x86_64-centos7-clang12-opt", "dev4/x86_64-centos7-gcc11-opt", "dev4/x86_64-centos7-clang12-opt"] + include: + - LCG: "LCG_102/x86_64-centos7-clang12-opt" + RNTUPLE: OFF + - LCG: "LCG_102/x86_64-centos8-gcc11-opt" + RNTUPLE: OFF steps: - uses: actions/checkout@v3 - uses: cvmfs-contrib/github-action-cvmfs@v3 @@ -29,7 +32,8 @@ jobs: echo "::group::Run CMake" mkdir build install cd build - cmake -DENABLE_SIO=${{ matrix.sio }} \ + cmake -DENABLE_SIO=ON \ + -DENABLE_RNTUPLE=${{ matrix.RNTUPLE }} \ -DCMAKE_INSTALL_PREFIX=../install \ -DCMAKE_CXX_STANDARD=17 \ -DCMAKE_CXX_FLAGS=" -fdiagnostics-color=always -Werror -Wno-error=deprecated-declarations " \ diff --git a/python/podio/root_io.py b/python/podio/root_io.py index 4dc6f16a7..a5f25950e 100644 --- a/python/podio/root_io.py +++ b/python/podio/root_io.py @@ -1,12 +1,12 @@ #!/usr/bin/env python3 """Python module for reading root files containing podio Frames""" -from podio.base_reader import BaseReaderMixin - from ROOT import gSystem gSystem.Load('libpodioRootIO') # noqa: E402 from ROOT import podio # noqa: E402 # pylint: disable=wrong-import-position +from podio.base_reader import BaseReaderMixin # pylint: disable=wrong-import-position + Writer = podio.ROOTFrameWriter diff --git a/python/podio/sio_io.py b/python/podio/sio_io.py index ae695fbb7..01f9d577f 100644 --- a/python/podio/sio_io.py +++ b/python/podio/sio_io.py @@ -1,8 +1,6 @@ #!/usr/bin/env python3 """Python module for reading sio files containing podio Frames""" -from podio.base_reader import BaseReaderMixin # pylint: disable=wrong-import-position - from ROOT import gSystem ret = gSystem.Load('libpodioSioIO') # noqa: 402 # Return values: -1 when it doesn't exist and -2 when there is a version mismatch @@ -10,6 +8,8 @@ raise ImportError('Error when importing libpodioSioIO') from ROOT import podio # noqa: 402 # pylint: disable=wrong-import-position +from podio.base_reader import BaseReaderMixin # pylint: disable=wrong-import-position + Writer = podio.SIOFrameWriter diff --git a/python/podio_class_generator.py b/python/podio_class_generator.py index 4f86f2a45..5f74a5ad6 100755 --- a/python/podio_class_generator.py +++ b/python/podio_class_generator.py @@ -13,9 +13,9 @@ import jinja2 +from podio_schema_evolution import DataModelComparator # dealing with cyclic imports from podio.podio_config_reader import PodioConfigReader from podio.generator_utils import DataType, DefinitionError, DataModelJSONEncoder -from podio_schema_evolution import DataModelComparator # dealing with cyclic imports THIS_DIR = os.path.dirname(os.path.abspath(__file__)) TEMPLATE_DIR = os.path.join(THIS_DIR, 'templates') diff --git a/src/EventStore.cc b/src/EventStore.cc index 947d83fff..831cc7030 100644 --- a/src/EventStore.cc +++ b/src/EventStore.cc @@ -34,7 +34,7 @@ bool EventStore::get(uint32_t id, CollectionBase*& collection) const { } void EventStore::registerCollection(const std::string& name, podio::CollectionBase* coll) { - m_collections.push_back({name, coll}); + m_collections.emplace_back(name, coll); auto id = m_table->add(name); coll->setID(id); } diff --git a/src/ROOTReader.cc b/src/ROOTReader.cc index 768e099f3..a366962ac 100644 --- a/src/ROOTReader.cc +++ b/src/ROOTReader.cc @@ -133,7 +133,7 @@ CollectionBase* ROOTReader::readCollectionData(const root_utils::CollectionBranc collection->setID(id); collection->prepareAfterRead(); - m_inputs.emplace_back(std::make_pair(collection, name)); + m_inputs.emplace_back(collection, name); return collection; } diff --git a/src/SIOBlockUserData.cc b/src/SIOBlockUserData.cc index 0c6a52381..e1318add2 100644 --- a/src/SIOBlockUserData.cc +++ b/src/SIOBlockUserData.cc @@ -1,7 +1,5 @@ #include "podio/SIOBlockUserData.h" -//#define PODIO_ADD_USER_TYPE_SIO(type) static UserDataSIOBlock _default##type##CollcetionSIOBlock ; - namespace podio { static SIOBlockUserData _defaultfloatCollcetionSIOBlock; @@ -18,22 +16,3 @@ static SIOBlockUserData _defaultuint32_tCollcetionSIOBlock; static SIOBlockUserData _defaultuint64_tCollcetionSIOBlock; } // namespace podio - -// g++ -E ../src/SIOBlockUserData.cc -// PODIO_ADD_USER_TYPE_SIO(int) -// PODIO_ADD_USER_TYPE_SIO(long) -// PODIO_ADD_USER_TYPE_SIO(float) -// PODIO_ADD_USER_TYPE_SIO(double) -// PODIO_ADD_USER_TYPE_SIO(unsigned) -// PODIO_ADD_USER_TYPE_SIO(unsigned int) -// PODIO_ADD_USER_TYPE_SIO(unsigned long) -// PODIO_ADD_USER_TYPE_SIO(char) -// PODIO_ADD_USER_TYPE_SIO(short) -// PODIO_ADD_USER_TYPE_SIO(long long) -// PODIO_ADD_USER_TYPE_SIO(unsigned long long) -// PODIO_ADD_USER_TYPE_SIO(int16_t) -// PODIO_ADD_USER_TYPE_SIO(int32_t) -// PODIO_ADD_USER_TYPE_SIO(int64_t) -// PODIO_ADD_USER_TYPE_SIO(uint16_t) -// PODIO_ADD_USER_TYPE_SIO(uint32_t) -// PODIO_ADD_USER_TYPE_SIO(uint64_t) diff --git a/src/SchemaEvolution.cc b/src/SchemaEvolution.cc index 2377e5833..60b53264e 100644 --- a/src/SchemaEvolution.cc +++ b/src/SchemaEvolution.cc @@ -50,7 +50,7 @@ void SchemaEvolution::registerEvolutionFunc(const std::string& collType, SchemaV // structure and update the index if (typeIt->second.index == MapIndex::NoEvolutionAvailable) { typeIt->second.index = m_evolutionFuncs.size(); - m_evolutionFuncs.emplace_back(EvolFuncVersionMapT{}); + m_evolutionFuncs.emplace_back(); } // From here on out we don't need the mutabale any longer From d2a895a1ae178d1ac270a2f66e355d95e0c83669 Mon Sep 17 00:00:00 2001 From: hegner Date: Thu, 13 Jul 2023 19:59:50 +0200 Subject: [PATCH 08/22] add description and author to components (#450) --- python/podio/podio_config_reader.py | 2 +- python/podio/test_ClassDefinitionValidator.py | 5 ----- python/templates/Component.h.jinja2 | 3 ++- tests/datalayout.yaml | 2 ++ 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/python/podio/podio_config_reader.py b/python/podio/podio_config_reader.py index c3d9b4106..4e99e2423 100644 --- a/python/podio/podio_config_reader.py +++ b/python/podio/podio_config_reader.py @@ -162,7 +162,7 @@ def _check_components(cls, datamodel, upstream_edm): """Check the components.""" for name, component in datamodel.components.items(): for field in component: - if field not in ['Members', 'ExtraCode']: + if field not in ['Members', 'ExtraCode', 'Description', 'Author']: raise DefinitionError(f"{name} defines a '{field}' field which is not allowed for a component") if 'ExtraCode' in component: diff --git a/python/podio/test_ClassDefinitionValidator.py b/python/podio/test_ClassDefinitionValidator.py index 5db93c195..c1b9d4cd1 100644 --- a/python/podio/test_ClassDefinitionValidator.py +++ b/python/podio/test_ClassDefinitionValidator.py @@ -109,11 +109,6 @@ def test_component_valid_members(self): self._assert_no_exception(DefinitionError, '{} should allow for component members in components', self.validate, make_dm(components, {}), False) - def test_component_invalid_field(self): - self.valid_component['Component']['Author'] = 'An invalid field for a component' - with self.assertRaises(DefinitionError): - self.validate(make_dm(self.valid_component, {}), False) - def test_datatype_valid_members(self): self._assert_no_exception(DefinitionError, '{} should not raise for a valid datatype', self.validate, make_dm({}, self.valid_datatype, self.def_opts)) diff --git a/python/templates/Component.h.jinja2 b/python/templates/Component.h.jinja2 index 2a9dad66a..bc82c3c92 100644 --- a/python/templates/Component.h.jinja2 +++ b/python/templates/Component.h.jinja2 @@ -1,3 +1,4 @@ +{% import "macros/declarations.jinja2" as macros %} {% import "macros/utils.jinja2" as utils %} // AUTOMATICALLY GENERATED FILE - DO NOT EDIT @@ -14,7 +15,7 @@ #endif {{ utils.namespace_open(class.namespace) }} - +{{ macros.class_description(class.bare_type, Description, Author) }} class {{ class.bare_type }} { public: {% for member in Members %} diff --git a/tests/datalayout.yaml b/tests/datalayout.yaml index 369d39b58..cfca90388 100755 --- a/tests/datalayout.yaml +++ b/tests/datalayout.yaml @@ -23,6 +23,8 @@ components : " NotSoSimpleStruct: + Description : "A not so simple struct" + Author : "Someone" Members: - SimpleStruct data // component members can have descriptions From e38f8386ed1fdb0e0fc081a975ba24bb7e7633d2 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 14 Jul 2023 08:14:46 +0200 Subject: [PATCH 09/22] Hide the JSON output functionality from CLING (#452) --- python/templates/Collection.cc.jinja2 | 2 +- python/templates/Collection.h.jinja2 | 4 ++-- python/templates/Component.h.jinja2 | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/python/templates/Collection.cc.jinja2 b/python/templates/Collection.cc.jinja2 index 88a18b305..99cbd01af 100644 --- a/python/templates/Collection.cc.jinja2 +++ b/python/templates/Collection.cc.jinja2 @@ -227,7 +227,7 @@ const auto registeredCollection = registerCollection(); } // namespace -#ifdef PODIO_JSON_OUTPUT +#if defined(PODIO_JSON_OUTPUT) && !defined(__CLING__) void to_json(nlohmann::json& j, const {{ collection_type }}& collection) { j = nlohmann::json::array(); for (auto&& elem : collection) { diff --git a/python/templates/Collection.h.jinja2 b/python/templates/Collection.h.jinja2 index 4ef0df066..24265eef1 100644 --- a/python/templates/Collection.h.jinja2 +++ b/python/templates/Collection.h.jinja2 @@ -18,7 +18,7 @@ #include "podio/CollectionBase.h" #include "podio/CollectionIDTable.h" -#ifdef PODIO_JSON_OUTPUT +#if defined(PODIO_JSON_OUTPUT) && !defined(__CLING__) #include "nlohmann/json.hpp" #endif @@ -193,7 +193,7 @@ Mutable{{ class.bare_type }} {{ class.bare_type }}Collection::create(Args&&... a return Mutable{{ class.bare_type }}(obj); } -#ifdef PODIO_JSON_OUTPUT +#if defined(PODIO_JSON_OUTPUT) && !defined(__CLING__) void to_json(nlohmann::json& j, const {{ class.bare_type }}Collection& collection); #endif diff --git a/python/templates/Component.h.jinja2 b/python/templates/Component.h.jinja2 index bc82c3c92..bc775b362 100644 --- a/python/templates/Component.h.jinja2 +++ b/python/templates/Component.h.jinja2 @@ -10,7 +10,7 @@ {% endfor %} #include -#ifdef PODIO_JSON_OUTPUT +#if defined(PODIO_JSON_OUTPUT) && !defined(__CLING__) #include "nlohmann/json.hpp" #endif @@ -40,7 +40,7 @@ inline std::ostream& operator<<(std::ostream& o, const {{class.full_type}}& valu return o; } -#ifdef PODIO_JSON_OUTPUT +#if defined(PODIO_JSON_OUTPUT) && !defined(__CLING__) inline void to_json(nlohmann::json& j, const {{ class.bare_type }}& value) { j = nlohmann::json{ {% set comma = joiner(",") %} From ac26a122b0446fee2dcd450a6d11625bf7ee9793 Mon Sep 17 00:00:00 2001 From: Juan Miguel Carceller <22276694+jmcarcell@users.noreply.github.com> Date: Fri, 14 Jul 2023 09:55:27 +0200 Subject: [PATCH 10/22] Rename CMAKE_{SOURCE,BIN}_DIR PROJECT_{SOURCE,BIN}_DIR (#454) --- cmake/podioCPack.cmake | 6 +++--- cmake/podioCreateConfig.cmake | 2 +- cmake/podioTest.cmake | 8 +++---- doc/Doxyfile.in | 6 +++--- python/CMakeLists.txt | 2 +- src/CMakeLists.txt | 38 +++++++++++++++++----------------- tests/CMakeLists.txt | 2 +- tests/dumpmodel/CMakeLists.txt | 20 +++++++++--------- 8 files changed, 42 insertions(+), 42 deletions(-) diff --git a/cmake/podioCPack.cmake b/cmake/podioCPack.cmake index 919acac7a..e20828ce4 100644 --- a/cmake/podioCPack.cmake +++ b/cmake/podioCPack.cmake @@ -15,9 +15,9 @@ set(CPACK_DEBIAN_PACKAGE_MAINTAINER "valentin.volkl@cern.ch") set(CPACK_DEBIAN_PACKAGE_HOMEPAGE ${CPACK_PACKAGE_HOMEPAGE_URL}) set(CPACK_DEBIAN_PACKAGE_RECOMMENDS "hep-root, python-yaml") -set(CPACK_PACKAGE_DESCRIPTION_FILE "${CMAKE_SOURCE_DIR}/README.md") -set(CPACK_RESOURCE_FILE_LICENSE "${CMAKE_SOURCE_DIR}/LICENSE") -set(CPACK_RESOURCE_FILE_README "${CMAKE_SOURCE_DIR}/README.md") +set(CPACK_PACKAGE_DESCRIPTION_FILE "${PROJECT_SOURCE_DIR}/README.md") +set(CPACK_RESOURCE_FILE_LICENSE "${PROJECT_SOURCE_DIR}/LICENSE") +set(CPACK_RESOURCE_FILE_README "${PROJECT_SOURCE_DIR}/README.md") #--- source package settings --------------------------------------------------- set(CPACK_SOURCE_IGNORE_FILES diff --git a/cmake/podioCreateConfig.cmake b/cmake/podioCreateConfig.cmake index 74d65e9a0..0d73cd441 100644 --- a/cmake/podioCreateConfig.cmake +++ b/cmake/podioCreateConfig.cmake @@ -19,7 +19,7 @@ configure_package_config_file(${PROJECT_SOURCE_DIR}/cmake/podioConfig.cmake.in install(FILES ${CMAKE_CURRENT_BINARY_DIR}/podioConfig.cmake ${CMAKE_CURRENT_BINARY_DIR}/podioConfigVersion.cmake - ${CMAKE_SOURCE_DIR}/cmake/podioMacros.cmake + ${PROJECT_SOURCE_DIR}/cmake/podioMacros.cmake DESTINATION ${CMAKE_INSTALL_CMAKEDIR}/${PROJECT_NAME} ) install(EXPORT podioTargets DESTINATION ${CMAKE_INSTALL_CMAKEDIR}/${PROJECT_NAME} diff --git a/cmake/podioTest.cmake b/cmake/podioTest.cmake index f668a8d7b..0515a45fa 100644 --- a/cmake/podioTest.cmake +++ b/cmake/podioTest.cmake @@ -6,13 +6,13 @@ function(PODIO_SET_TEST_ENV test) set_property(TEST ${test} PROPERTY ENVIRONMENT LD_LIBRARY_PATH=${CMAKE_BINARY_DIR}/tests:${CMAKE_BINARY_DIR}/src:$:$<$:$>:$ENV{LD_LIBRARY_PATH} - PYTHONPATH=${CMAKE_SOURCE_DIR}/python:$ENV{PYTHONPATH} + PYTHONPATH=${PROJECT_SOURCE_DIR}/python:$ENV{PYTHONPATH} PODIO_SIOBLOCK_PATH=${CMAKE_BINARY_DIR}/tests - ROOT_INCLUDE_PATH=${CMAKE_BINARY_DIR}/tests/datamodel:${CMAKE_SOURCE_DIR}/include + ROOT_INCLUDE_PATH=${CMAKE_BINARY_DIR}/tests/datamodel:${PROJECT_SOURCE_DIR}/include SKIP_SIO_TESTS=$> IO_HANDLERS=${IO_HANDLERS} PODIO_USE_CLANG_FORMAT=${PODIO_USE_CLANG_FORMAT} - PODIO_BASE=${CMAKE_SOURCE_DIR} + PODIO_BASE=${PROJECT_SOURCE_DIR} ENABLE_SIO=${ENABLE_SIO} ) endfunction() @@ -35,7 +35,7 @@ macro(PODIO_DOWNLOAD_LEGACY_INPUTS legacy_versions) if (NOT DEFINED CACHE{PODIO_TEST_INPUT_DATA_DIR} OR NOT EXISTS ${PODIO_TEST_INPUT_DATA_DIR}/v00-16-05/example_frame.root) message(STATUS "Getting test input files") execute_process( - COMMAND bash ${CMAKE_SOURCE_DIR}/tests/scripts/get_test_inputs.sh ${legacy_versions} + COMMAND bash ${PROJECT_SOURCE_DIR}/tests/scripts/get_test_inputs.sh ${legacy_versions} OUTPUT_VARIABLE podio_test_input_data_dir RESULT_VARIABLE test_inputs_available ) diff --git a/doc/Doxyfile.in b/doc/Doxyfile.in index 2de868d7e..b0f601f8f 100644 --- a/doc/Doxyfile.in +++ b/doc/Doxyfile.in @@ -144,7 +144,7 @@ FULL_PATH_NAMES = YES # will be relative from the directory where doxygen is started. # This tag requires that the tag FULL_PATH_NAMES is set to YES. -STRIP_FROM_PATH = @CMAKE_SOURCE_DIR@ @CMAKE_BINARY_DIR@ +STRIP_FROM_PATH = @PROJECT_SOURCE_DIR@ @CMAKE_BINARY_DIR@ # The STRIP_FROM_INC_PATH tag can be used to strip a user-defined part of the # path mentioned in the documentation of a class, which tells the reader which @@ -743,7 +743,7 @@ WARN_LOGFILE = @CMAKE_BINARY_DIR@/doxygen-warnings.log # spaces. # Note: If this tag is empty the current directory is searched. -INPUT = @CMAKE_SOURCE_DIR@ +INPUT = @PROJECT_SOURCE_DIR@ INPUT += @CMAKE_BINARY_DIR@/include INPUT += @CMAKE_CURRENT_BINARY_DIR@ @@ -818,7 +818,7 @@ EXCLUDE_SYMBOLS = # that contain example code fragments that are included (see the \include # command). -EXAMPLE_PATH = @CMAKE_SOURCE_DIR@ +EXAMPLE_PATH = @PROJECT_SOURCE_DIR@ # If the value of the EXAMPLE_PATH tag contains directories, you can use the # EXAMPLE_PATTERNS tag to specify one or more wildcard pattern (like *.cpp and diff --git a/python/CMakeLists.txt b/python/CMakeLists.txt index 9a7e39bd7..eea3a105a 100644 --- a/python/CMakeLists.txt +++ b/python/CMakeLists.txt @@ -28,7 +28,7 @@ install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/templates DESTINATION ${podio_PYTHON_INSTALLDIR}) IF (BUILD_TESTING) - add_test( NAME pyunittest COMMAND python3 -m unittest discover -s ${CMAKE_SOURCE_DIR}/python/podio) + add_test( NAME pyunittest COMMAND python3 -m unittest discover -s ${PROJECT_SOURCE_DIR}/python/podio) PODIO_SET_TEST_ENV(pyunittest) set_property(TEST pyunittest PROPERTY DEPENDS write write_frame_root) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index daea12e5f..8cdd0d813 100755 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -61,16 +61,16 @@ SET(core_sources ) SET(core_headers - ${CMAKE_SOURCE_DIR}/include/podio/CollectionBase.h - ${CMAKE_SOURCE_DIR}/include/podio/CollectionIDTable.h - ${CMAKE_SOURCE_DIR}/include/podio/EventStore.h - ${CMAKE_SOURCE_DIR}/include/podio/ICollectionProvider.h - ${CMAKE_SOURCE_DIR}/include/podio/IReader.h - ${CMAKE_SOURCE_DIR}/include/podio/ObjectID.h - ${CMAKE_SOURCE_DIR}/include/podio/UserDataCollection.h - ${CMAKE_SOURCE_DIR}/include/podio/podioVersion.h - ${CMAKE_SOURCE_DIR}/include/podio/DatamodelRegistry.h - ${CMAKE_SOURCE_DIR}/include/podio/utilities/DatamodelRegistryIOHelpers.h + ${PROJECT_SOURCE_DIR}/include/podio/CollectionBase.h + ${PROJECT_SOURCE_DIR}/include/podio/CollectionIDTable.h + ${PROJECT_SOURCE_DIR}/include/podio/EventStore.h + ${PROJECT_SOURCE_DIR}/include/podio/ICollectionProvider.h + ${PROJECT_SOURCE_DIR}/include/podio/IReader.h + ${PROJECT_SOURCE_DIR}/include/podio/ObjectID.h + ${PROJECT_SOURCE_DIR}/include/podio/UserDataCollection.h + ${PROJECT_SOURCE_DIR}/include/podio/podioVersion.h + ${PROJECT_SOURCE_DIR}/include/podio/DatamodelRegistry.h + ${PROJECT_SOURCE_DIR}/include/podio/utilities/DatamodelRegistryIOHelpers.h ) PODIO_ADD_LIB_AND_DICT(podio "${core_headers}" "${core_sources}" selection.xml) @@ -94,14 +94,14 @@ if(ENABLE_RNTUPLE) endif() SET(root_headers - ${CMAKE_SOURCE_DIR}/include/podio/ROOTFrameReader.h - ${CMAKE_SOURCE_DIR}/include/podio/ROOTLegacyReader.h - ${CMAKE_SOURCE_DIR}/include/podio/ROOTFrameWriter.h + ${PROJECT_SOURCE_DIR}/include/podio/ROOTFrameReader.h + ${PROJECT_SOURCE_DIR}/include/podio/ROOTLegacyReader.h + ${PROJECT_SOURCE_DIR}/include/podio/ROOTFrameWriter.h ) if(ENABLE_RNTUPLE) list(APPEND root_headers - ${CMAKE_SOURCE_DIR}/include/podio/ROOTNTupleReader.h - ${CMAKE_SOURCE_DIR}/include/podio/ROOTNTupleWriter.h + ${PROJECT_SOURCE_DIR}/include/podio/ROOTNTupleReader.h + ${PROJECT_SOURCE_DIR}/include/podio/ROOTNTupleWriter.h ) endif() @@ -119,7 +119,7 @@ SET(python_sources ) SET(python_headers - ${CMAKE_SOURCE_DIR}/include/podio/PythonEventStore.h + ${PROJECT_SOURCE_DIR}/include/podio/PythonEventStore.h ) PODIO_ADD_LIB_AND_DICT(podioPythonStore "${python_headers}" "${python_sources}" python_selection.xml) target_link_libraries(podioPythonStore PUBLIC podio::podio) @@ -140,9 +140,9 @@ if(ENABLE_SIO) ) SET(sio_headers - ${CMAKE_SOURCE_DIR}/include/podio/SIOFrameReader.h - ${CMAKE_SOURCE_DIR}/include/podio/SIOLegacyReader.h - ${CMAKE_SOURCE_DIR}/include/podio/SIOFrameWriter.h + ${PROJECT_SOURCE_DIR}/include/podio/SIOFrameReader.h + ${PROJECT_SOURCE_DIR}/include/podio/SIOLegacyReader.h + ${PROJECT_SOURCE_DIR}/include/podio/SIOFrameWriter.h ) PODIO_ADD_LIB_AND_DICT(podioSioIO "${sio_headers}" "${sio_sources}" sio_selection.xml) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 6591196c8..60b2bdd17 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -6,7 +6,7 @@ foreach( _conf ${CMAKE_CONFIGURATION_TYPES} ) endforeach() # Set the podio_PYTHON_DIR manually here because the macros below expect it -SET(podio_PYTHON_DIR ${CMAKE_SOURCE_DIR}/python) +SET(podio_PYTHON_DIR ${PROJECT_SOURCE_DIR}/python) PODIO_GENERATE_DATAMODEL(datamodel datalayout.yaml headers sources IO_BACKEND_HANDLERS ${PODIO_IO_HANDLERS} diff --git a/tests/dumpmodel/CMakeLists.txt b/tests/dumpmodel/CMakeLists.txt index ccab43bc6..d796325cb 100644 --- a/tests/dumpmodel/CMakeLists.txt +++ b/tests/dumpmodel/CMakeLists.txt @@ -1,19 +1,19 @@ # Add tests for storing and retrieving the EDM definitions into the produced # files -add_test(NAME datamodel_def_store_roundtrip_root COMMAND ${CMAKE_SOURCE_DIR}/tests/scripts/dumpModelRoundTrip.sh +add_test(NAME datamodel_def_store_roundtrip_root COMMAND ${PROJECT_SOURCE_DIR}/tests/scripts/dumpModelRoundTrip.sh ${CMAKE_BINARY_DIR}/tests/root_io/example_frame.root datamodel - ${CMAKE_SOURCE_DIR}/tests + ${PROJECT_SOURCE_DIR}/tests ) PODIO_SET_TEST_ENV(datamodel_def_store_roundtrip_root) # The extension model needs to know about the upstream model for generation add_test(NAME datamodel_def_store_roundtrip_root_extension COMMAND - ${CMAKE_SOURCE_DIR}/tests/scripts/dumpModelRoundTrip.sh + ${PROJECT_SOURCE_DIR}/tests/scripts/dumpModelRoundTrip.sh ${CMAKE_BINARY_DIR}/tests/root_io/example_frame.root extension_model - ${CMAKE_SOURCE_DIR}/tests/extension_model - --upstream-edm=datamodel:${CMAKE_SOURCE_DIR}/tests/datalayout.yaml + ${PROJECT_SOURCE_DIR}/tests/extension_model + --upstream-edm=datamodel:${PROJECT_SOURCE_DIR}/tests/datalayout.yaml ) PODIO_SET_TEST_ENV(datamodel_def_store_roundtrip_root_extension) @@ -27,18 +27,18 @@ set_tests_properties( set(sio_roundtrip_tests "") if (ENABLE_SIO) - add_test(NAME datamodel_def_store_roundtrip_sio COMMAND ${CMAKE_SOURCE_DIR}/tests/scripts/dumpModelRoundTrip.sh + add_test(NAME datamodel_def_store_roundtrip_sio COMMAND ${PROJECT_SOURCE_DIR}/tests/scripts/dumpModelRoundTrip.sh ${CMAKE_BINARY_DIR}/tests/sio_io/example_frame.sio datamodel - ${CMAKE_SOURCE_DIR}/tests + ${PROJECT_SOURCE_DIR}/tests ) PODIO_SET_TEST_ENV(datamodel_def_store_roundtrip_sio) # The extension model needs to know about the upstream model for generation - add_test(NAME datamodel_def_store_roundtrip_sio_extension COMMAND ${CMAKE_SOURCE_DIR}/tests/scripts/dumpModelRoundTrip.sh + add_test(NAME datamodel_def_store_roundtrip_sio_extension COMMAND ${PROJECT_SOURCE_DIR}/tests/scripts/dumpModelRoundTrip.sh ${CMAKE_BINARY_DIR}/tests/sio_io/example_frame.sio extension_model - ${CMAKE_SOURCE_DIR}/tests/extension_model - --upstream-edm=datamodel:${CMAKE_SOURCE_DIR}/tests/datalayout.yaml + ${PROJECT_SOURCE_DIR}/tests/extension_model + --upstream-edm=datamodel:${PROJECT_SOURCE_DIR}/tests/datalayout.yaml ) PODIO_SET_TEST_ENV(datamodel_def_store_roundtrip_sio_extension) From ace49f71ff16a4475bc16a2b3edaf01e730dcb9d Mon Sep 17 00:00:00 2001 From: Juan Miguel Carceller <22276694+jmcarcell@users.noreply.github.com> Date: Tue, 18 Jul 2023 11:33:52 +0200 Subject: [PATCH 11/22] Rename CMAKE_BINARY_DIR to PROJECT_BINARY_DIR (#455) Co-authored-by: jmcarcell --- cmake/podioDoxygen.cmake | 2 +- cmake/podioMacros.cmake | 2 +- cmake/podioTest.cmake | 6 +++--- doc/Doxyfile.in | 12 ++++++------ python/CMakeLists.txt | 2 +- tests/CMakeLists.txt | 2 +- tests/dumpmodel/CMakeLists.txt | 8 ++++---- tests/root_io/CMakeLists.txt | 2 +- tests/unittests/CMakeLists.txt | 2 +- tools/CMakeLists.txt | 16 ++++++++-------- 10 files changed, 27 insertions(+), 27 deletions(-) diff --git a/cmake/podioDoxygen.cmake b/cmake/podioDoxygen.cmake index 4a472b154..c8e09b9e1 100644 --- a/cmake/podioDoxygen.cmake +++ b/cmake/podioDoxygen.cmake @@ -8,7 +8,7 @@ if(DOXYGEN_FOUND) configure_file(${CMAKE_CURRENT_SOURCE_DIR}/doc/Doxyfile.in ${CMAKE_CURRENT_BINARY_DIR}/Doxyfile @ONLY) configure_file(${CMAKE_CURRENT_SOURCE_DIR}/doc/doxy-boot.js.in - ${CMAKE_BINARY_DIR}/doxygen/html/doxy-boot.js) + ${PROJECT_BINARY_DIR}/doxygen/html/doxy-boot.js) add_custom_target(doc ${DOXYGEN_EXECUTABLE} ${CMAKE_CURRENT_BINARY_DIR}/Doxyfile WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} diff --git a/cmake/podioMacros.cmake b/cmake/podioMacros.cmake index bb217801c..cf8124d18 100644 --- a/cmake/podioMacros.cmake +++ b/cmake/podioMacros.cmake @@ -368,7 +368,7 @@ function(PODIO_CHECK_CPP_FS FS_LIBS) # After that it should be built-in FOREACH(FS_LIB_NAME "" stdc++fs c++fs) # MESSAGE(STATUS "Linking against ${FS_LIB_NAME}") - try_compile(have_filesystem ${CMAKE_BINARY_DIR}/try ${PROJECT_SOURCE_DIR}/cmake/try_filesystem.cpp + try_compile(have_filesystem ${PROJECT_BINARY_DIR}/try ${PROJECT_SOURCE_DIR}/cmake/try_filesystem.cpp CXX_STANDARD ${CMAKE_CXX_STANDARD} CXX_EXTENSIONS False OUTPUT_VARIABLE HAVE_FS_OUTPUT diff --git a/cmake/podioTest.cmake b/cmake/podioTest.cmake index 0515a45fa..5230af7a3 100644 --- a/cmake/podioTest.cmake +++ b/cmake/podioTest.cmake @@ -5,10 +5,10 @@ function(PODIO_SET_TEST_ENV test) list(JOIN PODIO_IO_HANDLERS " " IO_HANDLERS) set_property(TEST ${test} PROPERTY ENVIRONMENT - LD_LIBRARY_PATH=${CMAKE_BINARY_DIR}/tests:${CMAKE_BINARY_DIR}/src:$:$<$:$>:$ENV{LD_LIBRARY_PATH} + LD_LIBRARY_PATH=${PROJECT_BINARY_DIR}/tests:${PROJECT_BINARY_DIR}/src:$:$<$:$>:$ENV{LD_LIBRARY_PATH} PYTHONPATH=${PROJECT_SOURCE_DIR}/python:$ENV{PYTHONPATH} - PODIO_SIOBLOCK_PATH=${CMAKE_BINARY_DIR}/tests - ROOT_INCLUDE_PATH=${CMAKE_BINARY_DIR}/tests/datamodel:${PROJECT_SOURCE_DIR}/include + PODIO_SIOBLOCK_PATH=${PROJECT_BINARY_DIR}/tests + ROOT_INCLUDE_PATH=${PROJECT_BINARY_DIR}/tests/datamodel:${PROJECT_SOURCE_DIR}/include SKIP_SIO_TESTS=$> IO_HANDLERS=${IO_HANDLERS} PODIO_USE_CLANG_FORMAT=${PODIO_USE_CLANG_FORMAT} diff --git a/doc/Doxyfile.in b/doc/Doxyfile.in index b0f601f8f..d64d9bcda 100644 --- a/doc/Doxyfile.in +++ b/doc/Doxyfile.in @@ -58,7 +58,7 @@ PROJECT_LOGO = # entered, it will be relative to the location where doxygen was started. If # left blank the current directory will be used. -OUTPUT_DIRECTORY = @CMAKE_BINARY_DIR@/doxygen +OUTPUT_DIRECTORY = @PROJECT_BINARY_DIR@/doxygen # If the CREATE_SUBDIRS tag is set to YES, then doxygen will create 4096 sub- # directories (in 2 levels) under the output directory of each output format and @@ -144,7 +144,7 @@ FULL_PATH_NAMES = YES # will be relative from the directory where doxygen is started. # This tag requires that the tag FULL_PATH_NAMES is set to YES. -STRIP_FROM_PATH = @PROJECT_SOURCE_DIR@ @CMAKE_BINARY_DIR@ +STRIP_FROM_PATH = @PROJECT_SOURCE_DIR@ @PROJECT_BINARY_DIR@ # The STRIP_FROM_INC_PATH tag can be used to strip a user-defined part of the # path mentioned in the documentation of a class, which tells the reader which @@ -153,7 +153,7 @@ STRIP_FROM_PATH = @PROJECT_SOURCE_DIR@ @CMAKE_BINARY_DIR@ # specify the list of include paths that are normally passed to the compiler # using the -I flag. -STRIP_FROM_INC_PATH = @DOXYGEN_INCLUDE_DIRS@ @CMAKE_BINARY_DIR@/include +STRIP_FROM_INC_PATH = @DOXYGEN_INCLUDE_DIRS@ @PROJECT_BINARY_DIR@/include # If the SHORT_NAMES tag is set to YES, doxygen will generate much shorter (but # less readable) file names. This can be useful is your file systems doesn't @@ -731,7 +731,7 @@ WARN_FORMAT = "$file:$line: $text" # messages should be written. If left blank the output is written to standard # error (stderr). -WARN_LOGFILE = @CMAKE_BINARY_DIR@/doxygen-warnings.log +WARN_LOGFILE = @PROJECT_BINARY_DIR@/doxygen-warnings.log #--------------------------------------------------------------------------- # Configuration options related to the input files @@ -744,7 +744,7 @@ WARN_LOGFILE = @CMAKE_BINARY_DIR@/doxygen-warnings.log # Note: If this tag is empty the current directory is searched. INPUT = @PROJECT_SOURCE_DIR@ -INPUT += @CMAKE_BINARY_DIR@/include +INPUT += @PROJECT_BINARY_DIR@/include INPUT += @CMAKE_CURRENT_BINARY_DIR@ # This tag can be used to specify the character encoding of the source files @@ -801,7 +801,7 @@ EXCLUDE_SYMLINKS = NO # Note that the wildcards are matched against the file with absolute path, so to # exclude all test directories for example use the pattern */test/* -EXCLUDE_PATTERNS = */tests/* */dict/* */cmake/* @CMAKE_BINARY_DIR@ +EXCLUDE_PATTERNS = */tests/* */dict/* */cmake/* @PROJECT_BINARY_DIR@ # The EXCLUDE_SYMBOLS tag can be used to specify one or more symbol names # (namespaces, classes, functions, etc.) that should be excluded from the diff --git a/python/CMakeLists.txt b/python/CMakeLists.txt index eea3a105a..a17b07748 100644 --- a/python/CMakeLists.txt +++ b/python/CMakeLists.txt @@ -35,5 +35,5 @@ IF (BUILD_TESTING) if (TARGET write_sio) set_property(TEST pyunittest PROPERTY DEPENDS write_sio write_frame_sio) endif() - set_property(TEST pyunittest PROPERTY WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/tests) + set_property(TEST pyunittest PROPERTY WORKING_DIRECTORY ${PROJECT_BINARY_DIR}/tests) ENDIF() diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 60b2bdd17..6e71fa2ee 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -63,4 +63,4 @@ CREATE_PODIO_TEST(ostream_operator.cpp "") CREATE_PODIO_TEST(write_ascii.cpp "") # Customize CTest to potentially disable some of the tests with known problems -configure_file(CTestCustom.cmake ${CMAKE_BINARY_DIR}/CTestCustom.cmake @ONLY) +configure_file(CTestCustom.cmake ${PROJECT_BINARY_DIR}/CTestCustom.cmake @ONLY) diff --git a/tests/dumpmodel/CMakeLists.txt b/tests/dumpmodel/CMakeLists.txt index d796325cb..345609210 100644 --- a/tests/dumpmodel/CMakeLists.txt +++ b/tests/dumpmodel/CMakeLists.txt @@ -1,7 +1,7 @@ # Add tests for storing and retrieving the EDM definitions into the produced # files add_test(NAME datamodel_def_store_roundtrip_root COMMAND ${PROJECT_SOURCE_DIR}/tests/scripts/dumpModelRoundTrip.sh - ${CMAKE_BINARY_DIR}/tests/root_io/example_frame.root + ${PROJECT_BINARY_DIR}/tests/root_io/example_frame.root datamodel ${PROJECT_SOURCE_DIR}/tests ) @@ -10,7 +10,7 @@ PODIO_SET_TEST_ENV(datamodel_def_store_roundtrip_root) # The extension model needs to know about the upstream model for generation add_test(NAME datamodel_def_store_roundtrip_root_extension COMMAND ${PROJECT_SOURCE_DIR}/tests/scripts/dumpModelRoundTrip.sh - ${CMAKE_BINARY_DIR}/tests/root_io/example_frame.root + ${PROJECT_BINARY_DIR}/tests/root_io/example_frame.root extension_model ${PROJECT_SOURCE_DIR}/tests/extension_model --upstream-edm=datamodel:${PROJECT_SOURCE_DIR}/tests/datalayout.yaml @@ -28,14 +28,14 @@ set_tests_properties( set(sio_roundtrip_tests "") if (ENABLE_SIO) add_test(NAME datamodel_def_store_roundtrip_sio COMMAND ${PROJECT_SOURCE_DIR}/tests/scripts/dumpModelRoundTrip.sh - ${CMAKE_BINARY_DIR}/tests/sio_io/example_frame.sio + ${PROJECT_BINARY_DIR}/tests/sio_io/example_frame.sio datamodel ${PROJECT_SOURCE_DIR}/tests ) PODIO_SET_TEST_ENV(datamodel_def_store_roundtrip_sio) # The extension model needs to know about the upstream model for generation add_test(NAME datamodel_def_store_roundtrip_sio_extension COMMAND ${PROJECT_SOURCE_DIR}/tests/scripts/dumpModelRoundTrip.sh - ${CMAKE_BINARY_DIR}/tests/sio_io/example_frame.sio + ${PROJECT_BINARY_DIR}/tests/sio_io/example_frame.sio extension_model ${PROJECT_SOURCE_DIR}/tests/extension_model --upstream-edm=datamodel:${PROJECT_SOURCE_DIR}/tests/datalayout.yaml diff --git a/tests/root_io/CMakeLists.txt b/tests/root_io/CMakeLists.txt index ad1537c23..bfa8309f3 100644 --- a/tests/root_io/CMakeLists.txt +++ b/tests/root_io/CMakeLists.txt @@ -54,7 +54,7 @@ if (DEFINED CACHE{PODIO_TEST_INPUT_DATA_DIR}) macro(ADD_PODIO_LEGACY_TEST version base_test input_file) add_test(NAME ${base_test}_${version} COMMAND ${base_test} ${PODIO_TEST_INPUT_DATA_DIR}/${version}/${input_file}) set_property(TEST ${base_test}_${version} PROPERTY ENVIRONMENT - LD_LIBRARY_PATH=${CMAKE_BINARY_DIR}/tests:${CMAKE_BINARY_DIR}/src:$ENV{LD_LIBRARY_PATH} + LD_LIBRARY_PATH=${PROJECT_BINARY_DIR}/tests:${PROJECT_BINARY_DIR}/src:$ENV{LD_LIBRARY_PATH} # Clear the ROOT_INCLUDE_PATH for the tests, to avoid potential conflicts # with existing headers from other installations ROOT_INCLUDE_PATH= diff --git a/tests/unittests/CMakeLists.txt b/tests/unittests/CMakeLists.txt index 1103de315..04f1e019a 100644 --- a/tests/unittests/CMakeLists.txt +++ b/tests/unittests/CMakeLists.txt @@ -83,7 +83,7 @@ else() WORKING_DIRECTORY ${CMAKE_CURRENT_LIST_DIR} TEST_PREFIX "UT_" # make it possible to filter easily with -R ^UT TEST_SPEC ${filter_tests} # discover only tests that are known to not fail - DL_PATHS ${CMAKE_CURRENT_BINARY_DIR}:${CMAKE_BINARY_DIR}/src:$:$<$:$>:$ENV{LD_LIBRARY_PATH} + DL_PATHS ${CMAKE_CURRENT_BINARY_DIR}:${PROJECT_BINARY_DIR}/src:$:$<$:$>:$ENV{LD_LIBRARY_PATH} PROPERTIES ENVIRONMENT PODIO_SIOBLOCK_PATH=${CMAKE_CURRENT_BINARY_DIR} diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt index 4c73df578..beb60e318 100644 --- a/tools/CMakeLists.txt +++ b/tools/CMakeLists.txt @@ -24,16 +24,16 @@ if(BUILD_TESTING) endfunction() CREATE_DUMP_TEST(podio-dump-help _dummy_target_ --help) - CREATE_DUMP_TEST(podio-dump-root-legacy "write" ${CMAKE_BINARY_DIR}/tests/root_io/example.root) - CREATE_DUMP_TEST(podio-dump-root "write_frame_root" ${CMAKE_BINARY_DIR}/tests/root_io/example_frame.root) - CREATE_DUMP_TEST(podio-dump-detailed-root "write_frame_root" --detailed --category other_events --entries 2:3 ${CMAKE_BINARY_DIR}/tests/root_io/example_frame.root) - CREATE_DUMP_TEST(podio-dump-detailed-root-legacy "write" --detailed --entries 2:3 ${CMAKE_BINARY_DIR}/tests/root_io/example.root) + CREATE_DUMP_TEST(podio-dump-root-legacy "write" ${PROJECT_BINARY_DIR}/tests/root_io/example.root) + CREATE_DUMP_TEST(podio-dump-root "write_frame_root" ${PROJECT_BINARY_DIR}/tests/root_io/example_frame.root) + CREATE_DUMP_TEST(podio-dump-detailed-root "write_frame_root" --detailed --category other_events --entries 2:3 ${PROJECT_BINARY_DIR}/tests/root_io/example_frame.root) + CREATE_DUMP_TEST(podio-dump-detailed-root-legacy "write" --detailed --entries 2:3 ${PROJECT_BINARY_DIR}/tests/root_io/example.root) if (ENABLE_SIO) - CREATE_DUMP_TEST(podio-dump-sio-legacy "write_sio" ${CMAKE_BINARY_DIR}/tests/sio_io/example.sio) - CREATE_DUMP_TEST(podio-dump-sio "write_frame_sio" --entries 4:7 ${CMAKE_BINARY_DIR}/tests/sio_io/example_frame.sio) - CREATE_DUMP_TEST(podio-dump-detailed-sio "write_frame_sio" --detailed --entries 9 ${CMAKE_BINARY_DIR}/tests/sio_io/example_frame.sio) - CREATE_DUMP_TEST(podio-dump-detailed-sio-legacy "write_sio" --detailed --entries 9 ${CMAKE_BINARY_DIR}/tests/sio_io/example.sio) + CREATE_DUMP_TEST(podio-dump-sio-legacy "write_sio" ${PROJECT_BINARY_DIR}/tests/sio_io/example.sio) + CREATE_DUMP_TEST(podio-dump-sio "write_frame_sio" --entries 4:7 ${PROJECT_BINARY_DIR}/tests/sio_io/example_frame.sio) + CREATE_DUMP_TEST(podio-dump-detailed-sio "write_frame_sio" --detailed --entries 9 ${PROJECT_BINARY_DIR}/tests/sio_io/example_frame.sio) + CREATE_DUMP_TEST(podio-dump-detailed-sio-legacy "write_sio" --detailed --entries 9 ${PROJECT_BINARY_DIR}/tests/sio_io/example.sio) endif() endif() From e0cb13569913b5d8a770f97f4ffa845776b66396 Mon Sep 17 00:00:00 2001 From: Juan Miguel Carceller <22276694+jmcarcell@users.noreply.github.com> Date: Tue, 18 Jul 2023 11:34:59 +0200 Subject: [PATCH 12/22] Cache podio_PYTHON_DIR (#456) Co-authored-by: jmcarcell --- tests/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 6e71fa2ee..ccb747007 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -6,7 +6,7 @@ foreach( _conf ${CMAKE_CONFIGURATION_TYPES} ) endforeach() # Set the podio_PYTHON_DIR manually here because the macros below expect it -SET(podio_PYTHON_DIR ${PROJECT_SOURCE_DIR}/python) +SET(podio_PYTHON_DIR ${PROJECT_SOURCE_DIR}/python CACHE PATH "Path to the podio python directory") PODIO_GENERATE_DATAMODEL(datamodel datalayout.yaml headers sources IO_BACKEND_HANDLERS ${PODIO_IO_HANDLERS} From 954ca264c9587687cbc363476e10447332c92037 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Tue, 18 Jul 2023 13:16:39 +0200 Subject: [PATCH 13/22] Codify some of the implicitly assumed conventions (#439) --- include/podio/Frame.h | 1 + include/podio/FrameCategories.h | 43 +++++++++++++++++++++++++++++++++ tests/read_frame.h | 18 +++++++------- tests/read_frame_auxiliary.h | 4 +-- tests/write_frame.h | 2 +- 5 files changed, 56 insertions(+), 12 deletions(-) create mode 100644 include/podio/FrameCategories.h diff --git a/include/podio/Frame.h b/include/podio/Frame.h index a71bba336..7b6f39c7a 100644 --- a/include/podio/Frame.h +++ b/include/podio/Frame.h @@ -4,6 +4,7 @@ #include "podio/CollectionBase.h" #include "podio/CollectionBufferFactory.h" #include "podio/CollectionIDTable.h" +#include "podio/FrameCategories.h" // mainly for convenience #include "podio/GenericParameters.h" #include "podio/ICollectionProvider.h" #include "podio/SchemaEvolution.h" diff --git a/include/podio/FrameCategories.h b/include/podio/FrameCategories.h new file mode 100644 index 000000000..321d4fc23 --- /dev/null +++ b/include/podio/FrameCategories.h @@ -0,0 +1,43 @@ +#ifndef PODIO_FRAMECATEGORIES_H +#define PODIO_FRAMECATEGORIES_H + +#include + +namespace podio { + +/** + * Create a parameterName that encodes the collection name and the parameter + * Name into one string. + * + * This codifies a convention that was decided on to store collection level + * parameters. These are parameters / metadata that are valid for all + * collections of a given name in a file, e.g. CellID encoding strings. These + * parameters are usually stored in a dedicated metadata Frame inside a file, + * see the predefined category names in the Cateogry namespace. + * + * @param collName the name of the collection + * @param paramName the name of the parameter + * + * @returns A single key string that combines the collection and parameter name + */ +inline std::string collMetadataParamName(const std::string& collName, const std::string& paramName) { + return collName + "__" + paramName; +} + +/** + * This namespace mimics an enum (at least in its usage) and simply defines + * either commonly used category names, or category names that form a + * convention. + */ +namespace Category { + /// The event category + constexpr const auto Event = "events"; + /// The run category + constexpr const auto Run = "runs"; + /// The metadata cateogry that is used to store a single Frame that holds data + /// that is valid for a whole file, e.g. collection level parameters + constexpr const auto Metadata = "metadata"; +} // namespace Category +} // namespace podio + +#endif diff --git a/tests/read_frame.h b/tests/read_frame.h index 69a48e341..6ec6423d7 100644 --- a/tests/read_frame.h +++ b/tests/read_frame.h @@ -77,13 +77,13 @@ int read_frames(const std::string& filename, bool assertBuildVersion = true) { return 1; } - if (reader.getEntries("events") != 10) { + if (reader.getEntries(podio::Category::Event) != 10) { std::cerr << "Could not read back the number of events correctly. " - << "(expected:" << 10 << ", actual: " << reader.getEntries("events") << ")" << std::endl; + << "(expected:" << 10 << ", actual: " << reader.getEntries(podio::Category::Event) << ")" << std::endl; return 1; } - if (reader.getEntries("events") != reader.getEntries("other_events")) { + if (reader.getEntries(podio::Category::Event) != reader.getEntries("other_events")) { std::cerr << "Could not read back the number of events correctly. " << "(expected:" << 10 << ", actual: " << reader.getEntries("other_events") << ")" << std::endl; return 1; @@ -91,8 +91,8 @@ int read_frames(const std::string& filename, bool assertBuildVersion = true) { // Read the frames in a different order than when writing them here to make // sure that the writing/reading order does not impose any usage requirements - for (size_t i = 0; i < reader.getEntries("events"); ++i) { - auto frame = podio::Frame(reader.readNextEntry("events")); + for (size_t i = 0; i < reader.getEntries(podio::Category::Event); ++i) { + auto frame = podio::Frame(reader.readNextEntry(podio::Category::Event)); if (reader.currentFileVersion() > podio::version::Version{0, 16, 2}) { if (frame.get("emptySubsetColl") == nullptr) { std::cerr << "Could not retrieve an empty subset collection" << std::endl; @@ -114,7 +114,7 @@ int read_frames(const std::string& filename, bool assertBuildVersion = true) { } } - if (reader.readNextEntry("events")) { + if (reader.readNextEntry(podio::Category::Event)) { std::cerr << "Trying to read more frame data than is present should return a nullptr" << std::endl; return 1; } @@ -127,10 +127,10 @@ int read_frames(const std::string& filename, bool assertBuildVersion = true) { // Reading specific (jumping to) entry { - auto frame = podio::Frame(reader.readEntry("events", 4)); + auto frame = podio::Frame(reader.readEntry(podio::Category::Event, 4)); processEvent(frame, 4, reader.currentFileVersion()); // Reading the next entry after jump, continues from after the jump - auto nextFrame = podio::Frame(reader.readNextEntry("events")); + auto nextFrame = podio::Frame(reader.readNextEntry(podio::Category::Event)); processEvent(nextFrame, 5, reader.currentFileVersion()); auto otherFrame = podio::Frame(reader.readEntry("other_events", 4)); @@ -147,7 +147,7 @@ int read_frames(const std::string& filename, bool assertBuildVersion = true) { } // Trying to read a Frame that is not present returns a nullptr - if (reader.readEntry("events", 10)) { + if (reader.readEntry(podio::Category::Event, 10)) { std::cerr << "Trying to read a specific entry that does not exist should return a nullptr" << std::endl; return 1; } diff --git a/tests/read_frame_auxiliary.h b/tests/read_frame_auxiliary.h index 8ae4ff1f7..e8dacbe89 100644 --- a/tests/read_frame_auxiliary.h +++ b/tests/read_frame_auxiliary.h @@ -56,9 +56,9 @@ int test_frame_aux_info(const std::string& fileName) { reader.openFile(fileName); // Test on the first event only here. Additionally, also only testing the - // "events" category, since that is the one where not all collections are + // events category, since that is the one where not all collections are // written - auto event = podio::Frame(reader.readEntry("events", 0)); + auto event = podio::Frame(reader.readEntry(podio::Category::Event, 0)); auto collsToRead = collsToWrite; if (reader.currentFileVersion() < podio::version::Version{0, 16, 3}) { diff --git a/tests/write_frame.h b/tests/write_frame.h index 8a0a2b3d3..e4405f988 100644 --- a/tests/write_frame.h +++ b/tests/write_frame.h @@ -424,7 +424,7 @@ void write_frames(const std::string& filename) { for (int i = 0; i < 10; ++i) { auto frame = makeFrame(i); - writer.writeFrame(frame, "events", collsToWrite); + writer.writeFrame(frame, podio::Category::Event, collsToWrite); } for (int i = 100; i < 110; ++i) { From 6b8eeb1824b0fb19f00b8d581b393282a5542be9 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Thu, 20 Jul 2023 14:07:26 +0200 Subject: [PATCH 14/22] Make building SIO tests depend on ENABLE_SIO (#457) --- tests/CMakeLists.txt | 4 ++- tests/sio_io/CMakeLists.txt | 54 +++++++++++++++++-------------------- 2 files changed, 28 insertions(+), 30 deletions(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index ccb747007..fe7451c9f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -54,7 +54,9 @@ add_executable(check_benchmark_outputs check_benchmark_outputs.cpp) target_link_libraries(check_benchmark_outputs PRIVATE ROOT::Tree) add_subdirectory(root_io) -add_subdirectory(sio_io) +if (ENABLE_SIO) + add_subdirectory(sio_io) +endif() add_subdirectory(unittests) add_subdirectory(dumpmodel) diff --git a/tests/sio_io/CMakeLists.txt b/tests/sio_io/CMakeLists.txt index ee9a80be7..6eecd6d90 100644 --- a/tests/sio_io/CMakeLists.txt +++ b/tests/sio_io/CMakeLists.txt @@ -1,32 +1,28 @@ -if (TARGET TestDataModelSioBlocks) - set(sio_dependent_tests - write_sio.cpp - read_sio.cpp - read_and_write_sio.cpp - write_timed_sio.cpp - read_timed_sio.cpp - read_frame_sio.cpp - write_frame_sio.cpp - read_frame_legacy_sio.cpp) - set(sio_libs podio::podioSioIO) - foreach( sourcefile ${sio_dependent_tests} ) - CREATE_PODIO_TEST(${sourcefile} "${sio_libs}") - endforeach() +set(sio_dependent_tests + write_sio.cpp + read_sio.cpp + read_and_write_sio.cpp + write_timed_sio.cpp + read_timed_sio.cpp + read_frame_sio.cpp + write_frame_sio.cpp + read_frame_legacy_sio.cpp) +set(sio_libs podio::podioSioIO) +foreach( sourcefile ${sio_dependent_tests} ) + CREATE_PODIO_TEST(${sourcefile} "${sio_libs}") +endforeach() - # These need to be linked against TTree explicitly, since it is not done - # through another library and the TimedReader/Writer decorators are - # header-only wrappers - target_link_libraries(write_timed_sio PRIVATE ROOT::Tree) - target_link_libraries(read_timed_sio PRIVATE ROOT::Tree) -endif() +# These need to be linked against TTree explicitly, since it is not done +# through another library and the TimedReader/Writer decorators are +# header-only wrappers +target_link_libraries(write_timed_sio PRIVATE ROOT::Tree) +target_link_libraries(read_timed_sio PRIVATE ROOT::Tree) -if (TARGET read_sio) - set_property(TEST read_sio PROPERTY DEPENDS write_sio) - set_property(TEST read_and_write_sio PROPERTY DEPENDS write_sio) - set_property(TEST read_timed_sio PROPERTY DEPENDS write_timed_sio) - set_property(TEST read_frame_sio PROPERTY DEPENDS write_frame_sio) - set_property(TEST read_frame_legacy_sio PROPERTY DEPENDS write_sio) +set_property(TEST read_sio PROPERTY DEPENDS write_sio) +set_property(TEST read_and_write_sio PROPERTY DEPENDS write_sio) +set_property(TEST read_timed_sio PROPERTY DEPENDS write_timed_sio) +set_property(TEST read_frame_sio PROPERTY DEPENDS write_frame_sio) +set_property(TEST read_frame_legacy_sio PROPERTY DEPENDS write_sio) - add_test(NAME check_benchmark_outputs_sio COMMAND check_benchmark_outputs write_benchmark_sio.root read_benchmark_sio.root) - set_property(TEST check_benchmark_outputs_sio PROPERTY DEPENDS read_timed_sio write_timed_sio) -endif() +add_test(NAME check_benchmark_outputs_sio COMMAND check_benchmark_outputs write_benchmark_sio.root read_benchmark_sio.root) +set_property(TEST check_benchmark_outputs_sio PROPERTY DEPENDS read_timed_sio write_timed_sio) From 25e5e3aa92d784e37c7ade6306451a3e4439fcf5 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Tue, 25 Jul 2023 13:28:57 +0200 Subject: [PATCH 15/22] Make sure that v00-16-06 files can be read with the master branch (#461) * Add v00-16-06 test files * Make the reader read v00-16-06 files correctly * Bump patch version to 99 for development --- CMakeLists.txt | 2 +- src/ROOTFrameReader.cc | 2 +- tests/CMakeLists.txt | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 35c521882..4f4ad10ae 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,7 +8,7 @@ project(podio) #--- Version ------------------------------------------------------------------- SET( ${PROJECT_NAME}_VERSION_MAJOR 0 ) SET( ${PROJECT_NAME}_VERSION_MINOR 16 ) -SET( ${PROJECT_NAME}_VERSION_PATCH 6 ) +SET( ${PROJECT_NAME}_VERSION_PATCH 99 ) SET( ${PROJECT_NAME}_VERSION "${${PROJECT_NAME}_VERSION_MAJOR}.${${PROJECT_NAME}_VERSION_MINOR}.${${PROJECT_NAME}_VERSION_PATCH}" ) diff --git a/src/ROOTFrameReader.cc b/src/ROOTFrameReader.cc index d7da9dfa9..f098ad204 100644 --- a/src/ROOTFrameReader.cc +++ b/src/ROOTFrameReader.cc @@ -155,7 +155,7 @@ void ROOTFrameReader::initCategory(CategoryInfo& catInfo, const std::string& cat // For backwards compatibility make it possible to read the index based files // from older versions - if (m_fileVersion <= podio::version::Version{0, 16, 5}) { + if (m_fileVersion <= podio::version::Version{0, 16, 6}) { std::tie(catInfo.branches, catInfo.storedClasses) = createCollectionBranchesIndexBased(catInfo.chain.get(), *catInfo.table, *collInfo); } else { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index fe7451c9f..bfd203fb4 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -45,6 +45,7 @@ set(legacy_test_versions v00-16 v00-16-02 v00-16-05 + v00-16-06 ) ### Define the actual tests From 65b58eac132f5685591028b9d103ffbaad9ca196 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Tue, 25 Jul 2023 14:22:47 +0200 Subject: [PATCH 16/22] Add python bindings for writing Frames (#447) * Add put method to Frame python bindings * Add a test that writes a file via the python bindings and ROOT * Add a c++ test that reads content written in python * Add a put_parameter method to the python Frame --- python/podio/base_writer.py | 25 +++++ python/podio/frame.py | 114 +++++++++++++++++++++-- python/podio/root_io.py | 14 ++- python/podio/sio_io.py | 14 ++- python/podio/test_Frame.py | 59 ++++++++++++ python/podio/test_utils.py | 54 ++++++++++- tests/CTestCustom.cmake | 4 + tests/read_python_frame.h | 106 +++++++++++++++++++++ tests/root_io/CMakeLists.txt | 7 ++ tests/root_io/read_python_frame_root.cpp | 7 ++ tests/root_io/write_frame_root.py | 7 ++ tests/sio_io/read_python_frame_sio.cpp | 7 ++ tests/sio_io/write_frame_sio.py | 7 ++ 13 files changed, 411 insertions(+), 14 deletions(-) create mode 100644 python/podio/base_writer.py create mode 100644 tests/read_python_frame.h create mode 100644 tests/root_io/read_python_frame_root.cpp create mode 100644 tests/root_io/write_frame_root.py create mode 100644 tests/sio_io/read_python_frame_sio.cpp create mode 100644 tests/sio_io/write_frame_sio.py diff --git a/python/podio/base_writer.py b/python/podio/base_writer.py new file mode 100644 index 000000000..6f2b5777a --- /dev/null +++ b/python/podio/base_writer.py @@ -0,0 +1,25 @@ +#!/usr/bin/env python3 +"""Python module for defining the basic writer interface that is used by the +backend specific bindings""" + + +class BaseWriterMixin: + """Mixin class that defines the base interface of the writers. + + The backend specific writers inherit from here and have to initialize the + following members: + - _writer: The actual writer that is able to write frames + """ + + def write_frame(self, frame, category, collections=None): + """Write the given frame under the passed category, optionally limiting the + collections that are written. + + Args: + frame (podio.frame.Frame): The Frame to write + category (str): The category name + collections (optional, default=None): The subset of collections to + write. If None, all collections are written + """ + # pylint: disable-next=protected-access + self._writer.writeFrame(frame._frame, category, collections or frame.collections) diff --git a/python/podio/frame.py b/python/podio/frame.py index 4822b00df..1a69d7a46 100644 --- a/python/podio/frame.py +++ b/python/podio/frame.py @@ -48,16 +48,38 @@ def _determine_cpp_type(idx_and_type): SUPPORTED_PARAMETER_TYPES = _determine_supported_parameter_types() -def _get_cpp_vector_types(type_str): - """Get the possible std::vector from the passed py_type string.""" - # Gather a list of all types that match the type_str (c++ or python) +def _get_cpp_types(type_str): + """Get all possible c++ types from the passed py_type string.""" types = list(filter(lambda t: type_str in t, SUPPORTED_PARAMETER_TYPES)) if not types: raise ValueError(f'{type_str} cannot be mapped to a valid parameter type') + return types + + +def _get_cpp_vector_types(type_str): + """Get the possible std::vector from the passed py_type string.""" + # Gather a list of all types that match the type_str (c++ or python) + types = _get_cpp_types(type_str) return [f'std::vector<{t}>' for t in map(lambda x: x[0], types)] +def _is_collection_base(thing): + """Check whether the passed thing is a podio::CollectionBase + + Args: + thing (any): any object + + Returns: + bool: True if thing is a base of podio::CollectionBase, False otherwise + """ + # Make sure to only instantiate the template with things that cppyy + # understands + if "cppyy" in repr(thing): + return cppyy.gbl.std.is_base_of[cppyy.gbl.podio.CollectionBase, type(thing)].value + return False + + class Frame: """Frame class that serves as a container of collection and meta data.""" @@ -78,17 +100,16 @@ def __init__(self, data=None): else: self._frame = podio.Frame() - self._collections = tuple(str(s) for s in self._frame.getAvailableCollections()) - self._param_key_types = self._init_param_keys() + self._param_key_types = self._get_param_keys_types() @property def collections(self): - """Get the available collection (names) from this Frame. + """Get the currently available collection (names) from this Frame. Returns: tuple(str): The names of the available collections from this Frame. """ - return self._collections + return tuple(str(s) for s in self._frame.getAvailableCollections()) def get(self, name): """Get a collection from the Frame by name. @@ -107,9 +128,32 @@ def get(self, name): raise KeyError(f"Collection '{name}' is not available") return collection + def put(self, collection, name): + """Put the collection into the frame + + The passed collectoin is "moved" into the Frame, i.e. it cannot be used any + longer after a call to this function. This also means that only objects that + were in the collection at the time of calling this function will be + available afterwards. + + Args: + collection (podio.CollectionBase): The collection to put into the Frame + name (str): The name of the collection + + Returns: + podio.CollectionBase: The reference to the collection that has been put + into the Frame. NOTE: That mutating this collection is not allowed. + + Raises: + ValueError: If collection is not actually a podio.CollectionBase + """ + if not _is_collection_base(collection): + raise ValueError("Can only put podio collections into a Frame") + return self._frame.put(cppyy.gbl.std.move(collection), name) + @property def parameters(self): - """Get the available parameter names from this Frame. + """Get the currently available parameter names from this Frame. Returns: tuple (str): The names of the available parameters from this Frame. @@ -163,6 +207,58 @@ def _get_param_value(par_type, name): return _get_param_value(vec_types[0], name) + def put_parameter(self, key, value, as_type=None): + """Put a parameter into the Frame. + + Puts a parameter into the Frame after doing some (incomplete) type checks. + If a list is passed the parameter type is determined from looking at the + first element of the list only. Additionally, since python doesn't + differentiate between floats and doubles, floats will always be stored as + doubles by default, use the as_type argument to change this if necessary. + + Args: + key (str): The name of the parameter + value (int, float, str or list of these): The parameter value + as_type (str, optional): Explicitly specify the type that should be used + to put the parameter into the Frame. Python types (e.g. "str") will + be converted to c++ types. This will override any automatic type + deduction that happens otherwise. Note that this will be taken at + pretty much face-value and there are only limited checks for this. + + Raises: + ValueError: If a non-supported parameter type is passed + """ + # For lists we determine the c++ vector type and use that to call the + # correct template overload explicitly + if isinstance(value, (list, tuple)): + type_name = as_type or type(value[0]).__name__ + vec_types = _get_cpp_vector_types(type_name) + if len(vec_types) == 0: + raise ValueError(f"Cannot put a parameter of type {type_name} into a Frame") + + par_type = vec_types[0] + if isinstance(value[0], float): + # Always store floats as doubles from the python side + par_type = par_type.replace("float", "double") + + self._frame.putParameter[par_type](key, value) + else: + if as_type is not None: + cpp_types = _get_cpp_types(as_type) + if len(cpp_types) == 0: + raise ValueError(f"Cannot put a parameter of type {as_type} into a Frame") + self._frame.putParameter[cpp_types[0]](key, value) + + # If we have a single integer, a std::string overload kicks in with higher + # priority than the template for some reason. So we explicitly select the + # correct template here + elif isinstance(value, int): + self._frame.putParameter["int"](key, value) + else: + self._frame.putParameter(key, value) + + self._param_key_types = self._get_param_keys_types() # refresh the cache + def get_parameters(self): """Get the complete podio::GenericParameters object stored in this Frame. @@ -200,7 +296,7 @@ def get_param_info(self, name): return par_infos - def _init_param_keys(self): + def _get_param_keys_types(self): """Initialize the param keys dict for easier lookup of the available parameters. Returns: diff --git a/python/podio/root_io.py b/python/podio/root_io.py index a5f25950e..9623ee24d 100644 --- a/python/podio/root_io.py +++ b/python/podio/root_io.py @@ -6,8 +6,7 @@ from ROOT import podio # noqa: E402 # pylint: disable=wrong-import-position from podio.base_reader import BaseReaderMixin # pylint: disable=wrong-import-position - -Writer = podio.ROOTFrameWriter +from podio.base_writer import BaseWriterMixin # pylint: disable=wrong-import-position class Reader(BaseReaderMixin): @@ -49,3 +48,14 @@ def __init__(self, filenames): self._is_legacy = True super().__init__() + + +class Writer(BaseWriterMixin): + """Writer class for writing podio root files""" + def __init__(self, filename): + """Create a writer for writing files + + Args: + filename (str): The name of the output file + """ + self._writer = podio.ROOTFrameWriter(filename) diff --git a/python/podio/sio_io.py b/python/podio/sio_io.py index 01f9d577f..30257a860 100644 --- a/python/podio/sio_io.py +++ b/python/podio/sio_io.py @@ -9,8 +9,7 @@ from ROOT import podio # noqa: 402 # pylint: disable=wrong-import-position from podio.base_reader import BaseReaderMixin # pylint: disable=wrong-import-position - -Writer = podio.SIOFrameWriter +from podio.base_writer import BaseWriterMixin # pylint: disable=wrong-import-position class Reader(BaseReaderMixin): @@ -46,3 +45,14 @@ def __init__(self, filename): self._is_legacy = True super().__init__() + + +class Writer(BaseWriterMixin): + """Writer class for writing podio root files""" + def __init__(self, filename): + """Create a writer for writing files + + Args: + filename (str): The name of the output file + """ + self._writer = podio.SIOFrameWriter(filename) diff --git a/python/podio/test_Frame.py b/python/podio/test_Frame.py index f8ec1ad96..c09143056 100644 --- a/python/podio/test_Frame.py +++ b/python/podio/test_Frame.py @@ -7,6 +7,8 @@ # using root_io as that should always be present regardless of which backends are built from podio.root_io import Reader +from podio.test_utils import ExampleHitCollection + # The expected collections in each frame EXPECTED_COLL_NAMES = { 'arrays', 'WithVectorMember', 'info', 'fixedWidthInts', 'mcparticles', @@ -34,6 +36,63 @@ def test_frame_invalid_access(self): with self.assertRaises(KeyError): _ = frame.get_parameter('NonExistantParameter') + with self.assertRaises(ValueError): + collection = [1, 2, 4] + _ = frame.put(collection, "invalid_collection_type") + + def test_frame_put_collection(self): + """Check that putting a collection works as expected""" + frame = Frame() + self.assertEqual(frame.collections, tuple()) + + hits = ExampleHitCollection() + hits.create() + hits2 = frame.put(hits, "hits_from_python") + self.assertEqual(frame.collections, tuple(["hits_from_python"])) + # The original collection is gone at this point, and ideally just leaves an + # empty shell + self.assertEqual(len(hits), 0) + # On the other hand the return value of put has the original content + self.assertEqual(len(hits2), 1) + + def test_frame_put_parameters(self): + """Check that putting a parameter works as expected""" + frame = Frame() + self.assertEqual(frame.parameters, tuple()) + + frame.put_parameter("a_string_param", "a string") + self.assertEqual(frame.parameters, tuple(["a_string_param"])) + self.assertEqual(frame.get_parameter("a_string_param"), "a string") + + frame.put_parameter("float_param", 3.14) + self.assertEqual(frame.get_parameter("float_param"), 3.14) + + frame.put_parameter("int", 42) + self.assertEqual(frame.get_parameter("int"), 42) + + frame.put_parameter("string_vec", ["a", "b", "cd"]) + str_vec = frame.get_parameter("string_vec") + self.assertEqual(len(str_vec), 3) + self.assertEqual(str_vec, ["a", "b", "cd"]) + + frame.put_parameter("more_ints", [1, 2345]) + int_vec = frame.get_parameter("more_ints") + self.assertEqual(len(int_vec), 2) + self.assertEqual(int_vec, [1, 2345]) + + frame.put_parameter("float_vec", [1.23, 4.56, 7.89]) + vec = frame.get_parameter("float_vec", as_type="double") + self.assertEqual(len(vec), 3) + self.assertEqual(vec, [1.23, 4.56, 7.89]) + + frame.put_parameter("real_float_vec", [1.23, 4.56, 7.89], as_type="float") + f_vec = frame.get_parameter("real_float_vec", as_type="float") + self.assertEqual(len(f_vec), 3) + self.assertEqual(vec, [1.23, 4.56, 7.89]) + + frame.put_parameter("float_as_float", 3.14, as_type="float") + self.assertAlmostEqual(frame.get_parameter("float_as_float"), 3.14, places=5) + class FrameReadTest(unittest.TestCase): """Unit tests for the Frame python bindings for Frames read from file. diff --git a/python/podio/test_utils.py b/python/podio/test_utils.py index 2c5e282b6..44efc9cce 100644 --- a/python/podio/test_utils.py +++ b/python/podio/test_utils.py @@ -2,5 +2,57 @@ """Utilities for python unittests""" import os +import ROOT +ROOT.gSystem.Load("libTestDataModelDict.so") # noqa: E402 +from ROOT import ExampleHitCollection, ExampleClusterCollection # noqa: E402 # pylint: disable=wrong-import-position -SKIP_SIO_TESTS = os.environ.get('SKIP_SIO_TESTS', '1') == '1' +from podio.frame import Frame # pylint: disable=wrong-import-position + + +SKIP_SIO_TESTS = os.environ.get("SKIP_SIO_TESTS", "1") == "1" + + +def create_hit_collection(): + """Create a simple hit collection with two hits for testing""" + hits = ExampleHitCollection() + hits.create(0xBAD, 0.0, 0.0, 0.0, 23.0) + hits.create(0xCAFFEE, 1.0, 0.0, 0.0, 12.0) + + return hits + + +def create_cluster_collection(): + """Create a simple cluster collection with two clusters""" + clusters = ExampleClusterCollection() + clu0 = clusters.create() + clu0.energy(3.14) + clu1 = clusters.create() + clu1.energy(1.23) + + return clusters + + +def create_frame(): + """Create a frame with an ExampleHit and an ExampleCluster collection""" + frame = Frame() + hits = create_hit_collection() + frame.put(hits, "hits_from_python") + clusters = create_cluster_collection() + frame.put(clusters, "clusters_from_python") + + frame.put_parameter("an_int", 42) + frame.put_parameter("some_floats", [1.23, 7.89, 3.14]) + frame.put_parameter("greetings", ["from", "python"]) + frame.put_parameter("real_float", 3.14, as_type="float") + frame.put_parameter("more_real_floats", [1.23, 4.56, 7.89], as_type="float") + + return frame + + +def write_file(writer_type, filename): + """Write a file using the given Writer type and put one Frame into it under + the events category + """ + writer = writer_type(filename) + event = create_frame() + writer.write_frame(event, "events") diff --git a/tests/CTestCustom.cmake b/tests/CTestCustom.cmake index b0e683f65..d4d05cd2a 100644 --- a/tests/CTestCustom.cmake +++ b/tests/CTestCustom.cmake @@ -22,6 +22,8 @@ if ((NOT "@FORCE_RUN_ALL_TESTS@" STREQUAL "ON") AND (NOT "@USE_SANITIZER@" STREQ read-legacy-files-root_v00-13 read_frame_legacy_root read_frame_root_multiple + write_python_frame_root + read_python_frame_root write_frame_root read_frame_root @@ -35,6 +37,8 @@ if ((NOT "@FORCE_RUN_ALL_TESTS@" STREQUAL "ON") AND (NOT "@USE_SANITIZER@" STREQ write_frame_sio read_frame_sio read_frame_legacy_sio + write_python_frame_sio + read_python_frame_sio write_ascii diff --git a/tests/read_python_frame.h b/tests/read_python_frame.h new file mode 100644 index 000000000..5a06cc4ce --- /dev/null +++ b/tests/read_python_frame.h @@ -0,0 +1,106 @@ +#ifndef PODIO_TESTS_READ_PYTHON_FRAME_H // NOLINT(llvm-header-guard): folder structure not suitable +#define PODIO_TESTS_READ_PYTHON_FRAME_H // NOLINT(llvm-header-guard): folder structure not suitable + +#include "datamodel/ExampleClusterCollection.h" +#include "datamodel/ExampleHitCollection.h" + +#include "podio/Frame.h" + +#include + +int checkHits(const ExampleHitCollection& hits) { + if (hits.size() != 2) { + std::cerr << "There should be two hits in the collection (actual size: " << hits.size() << ")" << std::endl; + return 1; + } + + auto hit1 = hits[0]; + if (hit1.cellID() != 0xbad || hit1.x() != 0.0 || hit1.y() != 0.0 || hit1.z() != 0.0 || hit1.energy() != 23.0) { + std::cerr << "Could not retrieve the correct hit[0]: (expected: " << ExampleHit(0xbad, 0.0, 0.0, 0.0, 23.0) + << ", actual: " << hit1 << ")" << std::endl; + return 1; + } + + auto hit2 = hits[1]; + if (hit2.cellID() != 0xcaffee || hit2.x() != 1.0 || hit2.y() != 0.0 || hit2.z() != 0.0 || hit2.energy() != 12.0) { + std::cerr << "Could not retrieve the correct hit[1]: (expected: " << ExampleHit(0xcaffee, 1.0, 0.0, 0.0, 12.0) + << ", actual: " << hit1 << ")" << std::endl; + return 1; + } + + return 0; +} + +int checkClusters(const ExampleClusterCollection& clusters) { + if (clusters.size() != 2) { + std::cerr << "There should be two clusters in the collection (actual size: " << clusters.size() << ")" << std::endl; + return 1; + } + + if (clusters[0].energy() != 3.14 || clusters[1].energy() != 1.23) { + std::cerr << "Energies of the clusters is wrong: (expected: 3.14 and 1.23, actual " << clusters[0].energy() + << " and " << clusters[1].energy() << ")" << std::endl; + return 1; + } + + return 0; +} + +template +std::ostream& operator<<(std::ostream& o, const std::vector& vec) { + auto delim = "["; + for (const auto& v : vec) { + o << std::exchange(delim, ", ") << v; + } + return o << "]"; +} + +int checkParameters(const podio::Frame& frame) { + const auto iVal = frame.getParameter("an_int"); + if (iVal != 42) { + std::cerr << "Parameter an_int was not stored correctly (expected 42, actual " << iVal << ")" << std::endl; + return 1; + } + + const auto& dVal = frame.getParameter>("some_floats"); + if (dVal.size() != 3 || dVal[0] != 1.23 || dVal[1] != 7.89 || dVal[2] != 3.14) { + std::cerr << "Parameter some_floats was not stored correctly (expected [1.23, 7.89, 3.14], actual " << dVal << ")" + << std::endl; + return 1; + } + + const auto& strVal = frame.getParameter>("greetings"); + if (strVal.size() != 2 || strVal[0] != "from" || strVal[1] != "python") { + std::cerr << "Parameter greetings was not stored correctly (expected [from, python], actual " << strVal << ")" + << std::endl; + return 1; + } + + const auto realFloat = frame.getParameter("real_float"); + if (realFloat != 3.14f) { + std::cerr << "Parameter real_float was not stored correctly (expected 3.14, actual " << realFloat << ")" + << std::endl; + return 1; + } + + const auto& realFloats = frame.getParameter>("more_real_floats"); + if (realFloats.size() != 3 || realFloats[0] != 1.23f || realFloats[1] != 4.56f || realFloats[2] != 7.89f) { + std::cerr << "Parameter more_real_floats was not stored as correctly (expected [1.23, 4.56, 7.89], actual" + << realFloats << ")" << std::endl; + } + + return 0; +} + +template +int read_frame(const std::string& filename) { + auto reader = ReaderT(); + reader.openFile(filename); + + auto event = podio::Frame(reader.readEntry("events", 0)); + + return checkHits(event.get("hits_from_python")) + + checkClusters(event.get("clusters_from_python")) + checkParameters(event); +} + +#endif // PODIO_TESTS_READ_PYTHON_FRAME_H diff --git a/tests/root_io/CMakeLists.txt b/tests/root_io/CMakeLists.txt index bfa8309f3..5c867a9fe 100644 --- a/tests/root_io/CMakeLists.txt +++ b/tests/root_io/CMakeLists.txt @@ -11,6 +11,7 @@ set(root_dependent_tests write_frame_root.cpp read_frame_legacy_root.cpp read_frame_root_multiple.cpp + read_python_frame_root.cpp ) if(ENABLE_RNTUPLE) set(root_dependent_tests @@ -69,3 +70,9 @@ if (DEFINED CACHE{PODIO_TEST_INPUT_DATA_DIR}) ADD_PODIO_LEGACY_TEST(${version} read_frame_legacy_root example.root legacy_test_cases) endforeach() endif() + +#--- Write via python and the ROOT backend and see if we can read it back in in +#--- c++ +add_test(NAME write_python_frame_root COMMAND python3 ${CMAKE_CURRENT_LIST_DIR}/write_frame_root.py) +PODIO_SET_TEST_ENV(write_python_frame_root) +set_property(TEST read_python_frame_root PROPERTY DEPENDS write_python_frame_root) diff --git a/tests/root_io/read_python_frame_root.cpp b/tests/root_io/read_python_frame_root.cpp new file mode 100644 index 000000000..23d1c0015 --- /dev/null +++ b/tests/root_io/read_python_frame_root.cpp @@ -0,0 +1,7 @@ +#include "read_python_frame.h" + +#include "podio/ROOTFrameReader.h" + +int main() { + return read_frame("example_frame_with_py.root"); +} diff --git a/tests/root_io/write_frame_root.py b/tests/root_io/write_frame_root.py new file mode 100644 index 000000000..38bece171 --- /dev/null +++ b/tests/root_io/write_frame_root.py @@ -0,0 +1,7 @@ +#!/usr/bin/env python3 +"""Script to write a Frame in ROOT format""" + +from podio import test_utils +from podio.root_io import Writer + +test_utils.write_file(Writer, "example_frame_with_py.root") diff --git a/tests/sio_io/read_python_frame_sio.cpp b/tests/sio_io/read_python_frame_sio.cpp new file mode 100644 index 000000000..61c3eb481 --- /dev/null +++ b/tests/sio_io/read_python_frame_sio.cpp @@ -0,0 +1,7 @@ +#include "read_python_frame.h" + +#include "podio/SIOFrameReader.h" + +int main() { + return read_frame("example_frame_with_py.sio"); +} diff --git a/tests/sio_io/write_frame_sio.py b/tests/sio_io/write_frame_sio.py new file mode 100644 index 000000000..94e08aa27 --- /dev/null +++ b/tests/sio_io/write_frame_sio.py @@ -0,0 +1,7 @@ +#!/usr/bin/env python3 +"""Script to write a Frame in SIO format""" + +from podio import test_utils +from podio.sio_io import Writer + +test_utils.write_file(Writer, "example_frame_with_py.sio") From dbf9425aa6d2b956c4c6fa49d45abe40cbe9e5af Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Wed, 26 Jul 2023 08:38:29 +0200 Subject: [PATCH 17/22] Fix I/O issues of subset collections with vector members (#463) * Add tests that reproduce crash / failure * Only recast vector members for non-subset collections * Only read / write vector member buffers if they exist with SIO * Make pyton tests aware of new collection * [format] flake8 fixes * Fix typo in test output * Cleanup unittests * Actually populate an empty test section * Add a test case show-casing the failure * Make sure to only access vector members if available * Add tests read and write tests for SIO * Add I/O tests to ignore list for sanitizer builds --- python/podio/test_Frame.py | 5 ++- python/templates/Collection.cc.jinja2 | 3 +- python/templates/CollectionData.cc.jinja2 | 8 +++-- python/templates/SIOBlock.cc.jinja2 | 20 ++++++----- python/templates/macros/sioblocks.jinja2 | 18 +++++----- tests/CTestCustom.cmake | 2 ++ tests/read_and_write_frame.h | 38 +++++++++++++++++++++ tests/read_frame.h | 13 +++++++ tests/root_io/CMakeLists.txt | 13 +++++-- tests/root_io/read_and_write_frame_root.cpp | 9 +++++ tests/sio_io/CMakeLists.txt | 14 ++++++-- tests/sio_io/read_and_write_frame_sio.cpp | 9 +++++ tests/unittests/unittest.cpp | 27 +++++++-------- tests/write_frame.h | 12 ++++++- 14 files changed, 149 insertions(+), 42 deletions(-) create mode 100644 tests/read_and_write_frame.h create mode 100644 tests/root_io/read_and_write_frame_root.cpp create mode 100644 tests/sio_io/read_and_write_frame_sio.cpp diff --git a/python/podio/test_Frame.py b/python/podio/test_Frame.py index c09143056..7761deaf0 100644 --- a/python/podio/test_Frame.py +++ b/python/podio/test_Frame.py @@ -18,7 +18,10 @@ 'emptyCollection', 'emptySubsetColl' } # The expected collections from the extension (only present in the other_events category) -EXPECTED_EXTENSION_COLL_NAMES = {"extension_Contained", "extension_ExternalComponent", "extension_ExternalRelation"} +EXPECTED_EXTENSION_COLL_NAMES = { + "extension_Contained", "extension_ExternalComponent", "extension_ExternalRelation", + "VectorMemberSubsetColl" + } # The expected parameter names in each frame EXPECTED_PARAM_NAMES = {'anInt', 'UserEventWeight', 'UserEventName', 'SomeVectorData', 'SomeValue'} diff --git a/python/templates/Collection.cc.jinja2 b/python/templates/Collection.cc.jinja2 index 99cbd01af..44b4c139d 100644 --- a/python/templates/Collection.cc.jinja2 +++ b/python/templates/Collection.cc.jinja2 @@ -198,14 +198,15 @@ podio::CollectionReadBuffers createBuffers(bool isSubset) { }; readBuffers.recast = [](podio::CollectionReadBuffers& buffers) { + // We only have any of these buffers if this is not a subset collection if (buffers.data) { buffers.data = podio::CollectionWriteBuffers::asVector<{{ class.full_type }}Data>(buffers.data); - } {% if VectorMembers %} {% for member in VectorMembers %} (*buffers.vectorMembers)[{{ loop.index0 }}].second = podio::CollectionWriteBuffers::asVector<{{ member.full_type }}>((*buffers.vectorMembers)[{{ loop.index0 }}].second); {% endfor %} {% endif %} + } }; return readBuffers; diff --git a/python/templates/CollectionData.cc.jinja2 b/python/templates/CollectionData.cc.jinja2 index 3946ad756..cfa143060 100644 --- a/python/templates/CollectionData.cc.jinja2 +++ b/python/templates/CollectionData.cc.jinja2 @@ -83,11 +83,13 @@ void {{ class_type }}::clear(bool isSubsetColl) { podio::CollectionWriteBuffers {{ class_type }}::getCollectionBuffers(bool isSubsetColl) { {% if VectorMembers %} - // Make sure these point to the right place, even if a collection has been - // moved since it has been created + if (!isSubsetColl) { + // Make sure these point to the right place, even if a collection has been + // moved since it has been created {% for member in VectorMembers %} - m_vecmem_info[{{ loop.index0 }}].second = &m_vec_{{ member.name }}; + m_vecmem_info[{{ loop.index0 }}].second = &m_vec_{{ member.name }}; {% endfor %} + } {% endif -%} return { diff --git a/python/templates/SIOBlock.cc.jinja2 b/python/templates/SIOBlock.cc.jinja2 index f8090ea75..4fc065b46 100644 --- a/python/templates/SIOBlock.cc.jinja2 +++ b/python/templates/SIOBlock.cc.jinja2 @@ -40,18 +40,18 @@ void {{ block_class }}::read(sio::read_device& device, sio::version_type version } {% if VectorMembers %} + if (not m_subsetColl) { {% for member in VectorMembers %} - // auto {{ member.name }}Buffers = new std::vector<{{ member.full_type }}>(); - // m_buffers.vectorMembers->emplace_back("{{ member.full_type }}", &{{ member.name }}Buffers); - m_buffers.vectorMembers->emplace_back("{{ member.full_type }}", new std::vector<{{ member.full_type }}>()); + m_buffers.vectorMembers->emplace_back("{{ member.full_type }}", new std::vector<{{ member.full_type }}>()); {% endfor %} - //---- read vector members - auto* vecMemInfo = m_buffers.vectorMembers; - unsigned size{0}; + //---- read vector members + auto* vecMemInfo = m_buffers.vectorMembers; + unsigned size{0}; {% for member in VectorMembers %} {{ macros.vector_member_read(member, loop.index0) }} {% endfor %} + } {% endif %} } @@ -72,13 +72,15 @@ void {{ block_class }}::write(sio::write_device& device) { } {% if VectorMembers %} - //---- write vector members - auto* vecMemInfo = m_buffers.vectorMembers; - unsigned size{0}; + if (not m_subsetColl) { + //---- write vector members + auto* vecMemInfo = m_buffers.vectorMembers; + unsigned size{0}; {% for member in VectorMembers %} {{ macros.vector_member_write(member, loop.index0) }} {% endfor %} + } {% endif %} } diff --git a/python/templates/macros/sioblocks.jinja2 b/python/templates/macros/sioblocks.jinja2 index b3b903f8b..e3893b142 100644 --- a/python/templates/macros/sioblocks.jinja2 +++ b/python/templates/macros/sioblocks.jinja2 @@ -1,16 +1,16 @@ {% macro vector_member_write(member, index) %} - auto* vec{{ index }} = *reinterpret_cast**>(vecMemInfo->at({{ index }}).second); - size = vec{{ index }}->size(); - device.data(size); - podio::handlePODDataSIO(device, &(*vec{{ index }})[0], size); + auto* vec{{ index }} = *reinterpret_cast**>(vecMemInfo->at({{ index }}).second); + size = vec{{ index }}->size(); + device.data(size); + podio::handlePODDataSIO(device, &(*vec{{ index }})[0], size); {% endmacro %} {% macro vector_member_read(member, index) %} - auto* vec{{ index }} = reinterpret_cast*>(vecMemInfo->at({{ index }}).second); - size = 0u; - device.data(size); - vec{{ index }}->resize(size); - podio::handlePODDataSIO(device, vec{{ index }}->data(), size); + auto* vec{{ index }} = reinterpret_cast*>(vecMemInfo->at({{ index }}).second); + size = 0u; + device.data(size); + vec{{ index }}->resize(size); + podio::handlePODDataSIO(device, vec{{ index }}->data(), size); {% endmacro %} diff --git a/tests/CTestCustom.cmake b/tests/CTestCustom.cmake index d4d05cd2a..59d9efa81 100644 --- a/tests/CTestCustom.cmake +++ b/tests/CTestCustom.cmake @@ -24,6 +24,7 @@ if ((NOT "@FORCE_RUN_ALL_TESTS@" STREQUAL "ON") AND (NOT "@USE_SANITIZER@" STREQ read_frame_root_multiple write_python_frame_root read_python_frame_root + read_and_write_frame_root write_frame_root read_frame_root @@ -39,6 +40,7 @@ if ((NOT "@FORCE_RUN_ALL_TESTS@" STREQUAL "ON") AND (NOT "@USE_SANITIZER@" STREQ read_frame_legacy_sio write_python_frame_sio read_python_frame_sio + read_and_write_frame_sio write_ascii diff --git a/tests/read_and_write_frame.h b/tests/read_and_write_frame.h new file mode 100644 index 000000000..b2d75e23c --- /dev/null +++ b/tests/read_and_write_frame.h @@ -0,0 +1,38 @@ +#ifndef PODIO_TESTS_READ_AND_WRITE_FRAME_H // NOLINT(llvm-header-guard): folder structure not suitable +#define PODIO_TESTS_READ_AND_WRITE_FRAME_H // NOLINT(llvm-header-guard): folder structure not suitable + +#include "read_frame.h" + +#include + +template +int rewrite_frames(const std::string& inputFile, const std::string& newOutput) { + auto reader = ReaderT(); + reader.openFile(inputFile); + + auto writer = WriterT(newOutput); + + const auto frame = podio::Frame(reader.readEntry(podio::Category::Event, 0)); + writer.writeFrame(frame, podio::Category::Event); + + const auto otherFrame = podio::Frame(reader.readEntry("other_events", 0)); + writer.writeFrame(otherFrame, "other_events"); + + return 0; +} + +template +int read_rewritten_frames(const std::string& inputName) { + auto reader = ReaderT(); + reader.openFile(inputName); + + const auto frame = podio::Frame(reader.readEntry(podio::Category::Event, 0)); + processEvent(frame, 0, reader.currentFileVersion()); + + const auto otherFrame = podio::Frame(reader.readEntry("other_events", 0)); + processEvent(otherFrame, 100, reader.currentFileVersion()); + + return 0; +} + +#endif // PODIO_TESTS_READ_AND_WRITE_FRAME_H diff --git a/tests/read_frame.h b/tests/read_frame.h index 6ec6423d7..321623b96 100644 --- a/tests/read_frame.h +++ b/tests/read_frame.h @@ -1,6 +1,7 @@ #ifndef PODIO_TESTS_READ_FRAME_H // NOLINT(llvm-header-guard): folder structure not suitable #define PODIO_TESTS_READ_FRAME_H // NOLINT(llvm-header-guard): folder structure not suitable +#include "datamodel/ExampleWithVectorMemberCollection.h" #include "read_test.h" #include "extension_model/ContainedTypeCollection.h" @@ -60,6 +61,14 @@ void processExtensions(const podio::Frame& event, int iEvent, podio::version::Ve ASSERT(structs[2].y == 2 * iEvent, "struct value not as expected"); } +void checkVecMemSubsetColl(const podio::Frame& event) { + const auto& subsetColl = event.get("VectorMemberSubsetColl"); + const auto& origColl = event.get("WithVectorMember"); + ASSERT(subsetColl.isSubsetCollection(), "subset collection not read back as a subset collection"); + ASSERT(subsetColl.size() == 1, "subset collection should have size 1"); + ASSERT(subsetColl[0] == origColl[0], "subset coll does not have the right contents"); +} + template int read_frames(const std::string& filename, bool assertBuildVersion = true) { auto reader = ReaderT(); @@ -112,6 +121,10 @@ int read_frames(const std::string& filename, bool assertBuildVersion = true) { if (reader.currentFileVersion() > podio::version::Version{0, 16, 2}) { processExtensions(otherFrame, i + 100, reader.currentFileVersion()); } + // As well as a test for the vector members subset category + if (reader.currentFileVersion() >= podio::version::Version{0, 16, 99}) { + checkVecMemSubsetColl(otherFrame); + } } if (reader.readNextEntry(podio::Category::Event)) { diff --git a/tests/root_io/CMakeLists.txt b/tests/root_io/CMakeLists.txt index 5c867a9fe..509749643 100644 --- a/tests/root_io/CMakeLists.txt +++ b/tests/root_io/CMakeLists.txt @@ -12,6 +12,7 @@ set(root_dependent_tests read_frame_legacy_root.cpp read_frame_root_multiple.cpp read_python_frame_root.cpp + read_and_write_frame_root.cpp ) if(ENABLE_RNTUPLE) set(root_dependent_tests @@ -33,8 +34,16 @@ set_property(TEST read-multiple PROPERTY DEPENDS write) set_property(TEST read_and_write PROPERTY DEPENDS write) set_property(TEST read_frame_legacy_root PROPERTY DEPENDS write) set_property(TEST read_timed PROPERTY DEPENDS write_timed) -set_property(TEST read_frame_root PROPERTY DEPENDS write_frame_root) -set_property(TEST read_frame_root_multiple PROPERTY DEPENDS write_frame_root) + +set_tests_properties( + read_frame_root + read_frame_root_multiple + read_and_write_frame_root + + PROPERTIES + DEPENDS write_frame_root +) + if(ENABLE_RNTUPLE) set_property(TEST read_rntuple PROPERTY DEPENDS write_rntuple) endif() diff --git a/tests/root_io/read_and_write_frame_root.cpp b/tests/root_io/read_and_write_frame_root.cpp new file mode 100644 index 000000000..c25840289 --- /dev/null +++ b/tests/root_io/read_and_write_frame_root.cpp @@ -0,0 +1,9 @@ +#include "read_and_write_frame.h" + +#include "podio/ROOTFrameReader.h" +#include "podio/ROOTFrameWriter.h" + +int main() { + return rewrite_frames("example_frame.root", "rewritten_frame.root") + + read_rewritten_frames("rewritten_frame.root"); +} diff --git a/tests/sio_io/CMakeLists.txt b/tests/sio_io/CMakeLists.txt index 6eecd6d90..c5a15b34f 100644 --- a/tests/sio_io/CMakeLists.txt +++ b/tests/sio_io/CMakeLists.txt @@ -6,7 +6,9 @@ set(sio_dependent_tests read_timed_sio.cpp read_frame_sio.cpp write_frame_sio.cpp - read_frame_legacy_sio.cpp) + read_frame_legacy_sio.cpp + read_and_write_frame_sio.cpp +) set(sio_libs podio::podioSioIO) foreach( sourcefile ${sio_dependent_tests} ) CREATE_PODIO_TEST(${sourcefile} "${sio_libs}") @@ -21,8 +23,16 @@ target_link_libraries(read_timed_sio PRIVATE ROOT::Tree) set_property(TEST read_sio PROPERTY DEPENDS write_sio) set_property(TEST read_and_write_sio PROPERTY DEPENDS write_sio) set_property(TEST read_timed_sio PROPERTY DEPENDS write_timed_sio) -set_property(TEST read_frame_sio PROPERTY DEPENDS write_frame_sio) set_property(TEST read_frame_legacy_sio PROPERTY DEPENDS write_sio) +set_tests_properties( + read_frame_sio + read_and_write_frame_sio + + PROPERTIES + DEPENDS + write_frame_sio +) + add_test(NAME check_benchmark_outputs_sio COMMAND check_benchmark_outputs write_benchmark_sio.root read_benchmark_sio.root) set_property(TEST check_benchmark_outputs_sio PROPERTY DEPENDS read_timed_sio write_timed_sio) diff --git a/tests/sio_io/read_and_write_frame_sio.cpp b/tests/sio_io/read_and_write_frame_sio.cpp new file mode 100644 index 000000000..27a2ae76c --- /dev/null +++ b/tests/sio_io/read_and_write_frame_sio.cpp @@ -0,0 +1,9 @@ +#include "read_and_write_frame.h" + +#include "podio/SIOFrameReader.h" +#include "podio/SIOFrameWriter.h" + +int main() { + return rewrite_frames("example_frame.sio", "rewritten_frame.sio") + + read_rewritten_frames("rewritten_frame.sio"); +} diff --git a/tests/unittests/unittest.cpp b/tests/unittests/unittest.cpp index 6903a2e2b..cc87410f7 100644 --- a/tests/unittests/unittest.cpp +++ b/tests/unittests/unittest.cpp @@ -773,20 +773,6 @@ TEST_CASE("Move-only collections", "[collections][move-semantics]") { auto newClusters = std::move(clusterColl); vecMemColl.prepareForWrite(); - auto buffers = vecMemColl.getBuffers(); - auto vecBuffers = buffers.vectorMembers; - auto thisVec = (*vecBuffers)[0].second; - - const auto floatVec = podio::CollectionWriteBuffers::asVector(thisVec); - const auto floatVec2 = podio::CollectionReadBuffers::asVector(thisVec); - - std::cout << floatVec->size() << '\n'; - std::cout << floatVec2->size() << '\n'; - - // auto vecBuffers = buffers.vectorMembers; - // const auto vecBuffer = podio::CollectionWriteBuffers::asVector((*vecBuffers)[0].second); - // TD td; - // REQUIRE(vecBuffer->size() == 2); auto newVecMems = std::move(vecMemColl); userDataColl.prepareForWrite(); @@ -796,6 +782,19 @@ TEST_CASE("Move-only collections", "[collections][move-semantics]") { } SECTION("Moved collections can be prepared") { + auto newHits = std::move(hitColl); + newHits.prepareForWrite(); + + auto newClusters = std::move(clusterColl); + newClusters.prepareForWrite(); + + auto newVecMems = std::move(vecMemColl); + newVecMems.prepareForWrite(); + + auto newUserData = std::move(userDataColl); + newUserData.prepareForWrite(); + + checkCollections(newHits, newClusters, newVecMems, newUserData); } SECTION("Prepared collections can be move assigned") { diff --git a/tests/write_frame.h b/tests/write_frame.h index e4405f988..d80871d2e 100644 --- a/tests/write_frame.h +++ b/tests/write_frame.h @@ -215,6 +215,15 @@ auto createVectorMemberCollection(int i) { return vecs; } +auto createVectorMemberSubsetCollection(const ExampleWithVectorMemberCollection& coll) { + ExampleWithVectorMemberCollection refs; + refs.setSubsetCollection(); + + refs.push_back(coll[0]); + + return refs; +} + auto createInfoCollection(int i) { EventInfoCollection info; @@ -361,7 +370,8 @@ podio::Frame makeFrame(int iFrame) { podio::Frame frame{}; frame.put(createArrayCollection(iFrame), "arrays"); - frame.put(createVectorMemberCollection(iFrame), "WithVectorMember"); + const auto& vecMemColl = frame.put(createVectorMemberCollection(iFrame), "WithVectorMember"); + frame.put(createVectorMemberSubsetCollection(vecMemColl), "VectorMemberSubsetColl"); frame.put(createInfoCollection(iFrame), "info"); frame.put(createFixedWidthCollection(), "fixedWidthInts"); From fe8481b68bb2ebc8e8b40abd250392717ada4745 Mon Sep 17 00:00:00 2001 From: Andre Sailer Date: Tue, 22 Aug 2023 08:41:09 +0200 Subject: [PATCH 18/22] CI: use clang16 nightly builds (#469) * CI: use clang16 nightly builds * CI: linux: add CXX_STANDARD variable and set for builds * Tests: require newer catch2 version when using c++20, get latest catch2 v3 version for internal * CI: key4hep use AUTO for catch2 * CI: disable key4hep release based build --- .github/workflows/key4hep.yml | 9 +++++---- .github/workflows/test.yml | 12 ++++++++---- tests/unittests/CMakeLists.txt | 11 ++++++++--- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/.github/workflows/key4hep.yml b/.github/workflows/key4hep.yml index 43f69ef67..5135b4741 100644 --- a/.github/workflows/key4hep.yml +++ b/.github/workflows/key4hep.yml @@ -13,8 +13,9 @@ jobs: strategy: fail-fast: false matrix: - release: ["sw.hsf.org/key4hep", - "sw-nightlies.hsf.org/key4hep"] + include: + - release: "sw-nightlies.hsf.org/key4hep" + RNTUPLE: ON steps: - uses: actions/checkout@v3 - uses: cvmfs-contrib/github-action-cvmfs@v3 @@ -30,8 +31,8 @@ jobs: -DCMAKE_INSTALL_PREFIX=../install \ -DCMAKE_CXX_STANDARD=17 \ -DCMAKE_CXX_FLAGS=" -fdiagnostics-color=always -Werror -Wno-error=deprecated-declarations " \ - -DUSE_EXTERNAL_CATCH2=ON \ - -DENABLE_RNTUPLE=ON \ + -DUSE_EXTERNAL_CATCH2=AUTO \ + -DENABLE_RNTUPLE=${{ matrix.RNTUPLE }} \ -G Ninja .. echo "::endgroup::" echo "::group::Build" diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 858d20849..3b30009df 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -13,15 +13,19 @@ jobs: strategy: fail-fast: false matrix: + LCG: ["dev3/x86_64-el9-clang16-opt", + "dev4/x86_64-el9-clang16-opt"] + CXX_STANDARD: [20] RNTUPLE: [ON] - LCG: ["dev3/x86_64-centos7-clang12-opt", - "dev4/x86_64-centos7-gcc11-opt", - "dev4/x86_64-centos7-clang12-opt"] include: + - LCG: "dev4/x86_64-centos7-gcc11-opt" + CXX_STANDARD: 17 - LCG: "LCG_102/x86_64-centos7-clang12-opt" RNTUPLE: OFF + CXX_STANDARD: 17 - LCG: "LCG_102/x86_64-centos8-gcc11-opt" RNTUPLE: OFF + CXX_STANDARD: 17 steps: - uses: actions/checkout@v3 - uses: cvmfs-contrib/github-action-cvmfs@v3 @@ -35,7 +39,7 @@ jobs: cmake -DENABLE_SIO=ON \ -DENABLE_RNTUPLE=${{ matrix.RNTUPLE }} \ -DCMAKE_INSTALL_PREFIX=../install \ - -DCMAKE_CXX_STANDARD=17 \ + -DCMAKE_CXX_STANDARD=${{ matrix.CXX_STANDARD }} \ -DCMAKE_CXX_FLAGS=" -fdiagnostics-color=always -Werror -Wno-error=deprecated-declarations " \ -DUSE_EXTERNAL_CATCH2=OFF \ -G Ninja .. diff --git a/tests/unittests/CMakeLists.txt b/tests/unittests/CMakeLists.txt index 04f1e019a..5d938ef6a 100644 --- a/tests/unittests/CMakeLists.txt +++ b/tests/unittests/CMakeLists.txt @@ -1,8 +1,13 @@ +if(CMAKE_CXX_STANDARD GREATER_EQUAL 20) + set(CATCH2_MIN_VERSION 3.4) +else() + set(CATCH2_MIN_VERSION 3.1) +endif() if(USE_EXTERNAL_CATCH2) if (USE_EXTERNAL_CATCH2 STREQUAL AUTO) - find_package(Catch2 3.1) + find_package(Catch2 ${CATCH2_MIN_VERSION}) else() - find_package(Catch2 3.1 REQUIRED) + find_package(Catch2 ${CATCH2_MIN_VERSION} REQUIRED) endif() endif() @@ -16,7 +21,7 @@ if(NOT Catch2_FOUND) FetchContent_Declare( Catch2 GIT_REPOSITORY https://github.com/catchorg/Catch2.git - GIT_TAG v3.1.0 + GIT_TAG v3.4.0 ) FetchContent_MakeAvailable(Catch2) set(CMAKE_MODULE_PATH ${Catch2_SOURCE_DIR}/extras ${CMAKE_MODULE_PATH}) From 60dcd12f062658cbd62442d83514b2eb9f140277 Mon Sep 17 00:00:00 2001 From: hegner Date: Tue, 22 Aug 2023 14:43:53 +0200 Subject: [PATCH 19/22] Allow to specify units (#445) * add regex to support unit definition * add tests * add unit syntax documentation * extend tests for members * add a component with unit * add units to generated documentation --- doc/datamodel_syntax.md | 7 +++++ python/podio/generator_utils.py | 17 +++++++++--- python/podio/podio_config_reader.py | 30 ++++++++++++--------- python/podio/test_MemberParser.py | 11 ++++++++ python/templates/macros/declarations.jinja2 | 16 +++++------ tests/datalayout.yaml | 18 ++++++------- 6 files changed, 66 insertions(+), 33 deletions(-) diff --git a/doc/datamodel_syntax.md b/doc/datamodel_syntax.md index e665860e6..b2ca2ff44 100644 --- a/doc/datamodel_syntax.md +++ b/doc/datamodel_syntax.md @@ -85,6 +85,13 @@ It is also possible to specify default values for members via Note that in this case it is extremely expensive to check whether the provided `default-value` results in valid c++. Hence, there is only a very basic syntax check, but no actual type check, and wrong default values will be caught only when trying to compile the datamodel. +For describing physics quantities it is important to know their units. Thus it is possible to add the units to the member definition: + +```yaml + Members: + {} [] // +``` + ### Definition of references between objects: There can be one-to-one-relations and one-to-many relations being stored in a particular class. This happens either in the `OneToOneRelations` or `OneToManyRelations` section of the data definition. The definition has again the form: diff --git a/python/podio/generator_utils.py b/python/podio/generator_utils.py index 17d3a37f3..6377aa09e 100644 --- a/python/podio/generator_utils.py +++ b/python/podio/generator_utils.py @@ -91,6 +91,7 @@ def __init__(self, name, **kwargs): self.full_type = kwargs.pop('type', '') self.description = kwargs.pop('description', '') self.default_val = kwargs.pop('default_val', None) + self.unit = kwargs.pop('unit', None) self.is_builtin = False self.is_builtin_array = False self.is_array = False @@ -150,6 +151,15 @@ def __init__(self, name, **kwargs): else: self.namespace, self.bare_type = _get_namespace_class(self.full_type) + @property + def docstring(self): + """Docstring to be used in code generation""" + if self.unit is not None: + docstring = rf'{self.description} [{self.unit}]' + else: + docstring = self.description + return docstring + def __str__(self): """string representation""" # Make sure to include scope-operator if necessary @@ -163,8 +173,8 @@ def __str__(self): else: definition = rf'{scoped_type} {self.name}{{}};' - if self.description: - definition += rf' ///< {self.description}' + if self.docstring: + definition += rf' ///< {self.docstring}' return definition def getter_name(self, get_syntax): @@ -190,7 +200,8 @@ def _to_json(self): # things again here from available information def_val = f'{{{self.default_val}}}' if self.default_val else '' description = f' // {self.description}' if self.description else '' - return f'{self.full_type} {self.name}{def_val}{description}' + unit = f'[{self.unit}]' if self.unit else '' + return f'{self.full_type} {self.name}{def_val}{unit}{description}' class DataModel: # pylint: disable=too-few-public-methods diff --git a/python/podio/podio_config_reader.py b/python/podio/podio_config_reader.py index 4e99e2423..303632367 100644 --- a/python/podio/podio_config_reader.py +++ b/python/podio/podio_config_reader.py @@ -30,6 +30,10 @@ class MemberParser: name_str = r'([a-zA-Z_]+\w*)' name_re = re.compile(name_str) + # Units are given in square brakets + unit_str = r'(?:\[([a-zA-Z_*\/]+\w*)\])?' + unit_re = re.compile(unit_str) + # Comments can be anything after // # stripping of trailing whitespaces is done later as it is hard to do with regex comment_str = r'\/\/ *(.*)' @@ -41,12 +45,12 @@ class MemberParser: def_val_str = r'(?:{(.+)})?' array_re = re.compile(array_str) - full_array_re = re.compile(rf'{array_str} *{name_str} *{def_val_str} *{comment_str}') - member_re = re.compile(rf' *{type_str} +{name_str} *{def_val_str} *{comment_str}') + full_array_re = re.compile(rf'{array_str} *{name_str} *{def_val_str} *{unit_str} *{comment_str}') + member_re = re.compile(rf' *{type_str} +{name_str} *{def_val_str} *{unit_str} *{comment_str}') # For cases where we don't require a description - bare_member_re = re.compile(rf' *{type_str} +{name_str} *{def_val_str}') - bare_array_re = re.compile(rf' *{array_str} +{name_str} *{def_val_str}') + bare_member_re = re.compile(rf' *{type_str} +{name_str} *{def_val_str} *{unit_str}') + bare_array_re = re.compile(rf' *{array_str} +{name_str} *{def_val_str} *{unit_str}') @staticmethod def _parse_with_regexps(string, regexps_callbacks): @@ -56,32 +60,32 @@ def _parse_with_regexps(string, regexps_callbacks): result = rgx.match(string) if result: return callback(result) - raise DefinitionError(f"'{string}' is not a valid member definition. Check syntax of the member definition.") @staticmethod def _full_array_conv(result): """MemberVariable construction for array members with a docstring""" - typ, size, name, def_val, comment = result.groups() - return MemberVariable(name=name, array_type=typ, array_size=size, description=comment.strip(), default_val=def_val) + typ, size, name, def_val, unit, comment = result.groups() + return MemberVariable(name=name, array_type=typ, array_size=size, description=comment.strip(), + unit=unit, default_val=def_val) @staticmethod def _full_member_conv(result): """MemberVariable construction for members with a docstring""" - klass, name, def_val, comment = result.groups() - return MemberVariable(name=name, type=klass, description=comment.strip(), default_val=def_val) + klass, name, def_val, unit, comment = result.groups() + return MemberVariable(name=name, type=klass, description=comment.strip(), unit=unit, default_val=def_val) @staticmethod def _bare_array_conv(result): """MemberVariable construction for array members without docstring""" - typ, size, name, def_val = result.groups() - return MemberVariable(name=name, array_type=typ, array_size=size, default_val=def_val) + typ, size, name, def_val, unit = result.groups() + return MemberVariable(name=name, array_type=typ, array_size=size, unit=unit, default_val=def_val) @staticmethod def _bare_member_conv(result): """MemberVarible construction for members without docstring""" - klass, name, def_val = result.groups() - return MemberVariable(name=name, type=klass, default_val=def_val) + klass, name, def_val, unit = result.groups() + return MemberVariable(name=name, type=klass, unit=unit, default_val=def_val) def parse(self, string, require_description=True): """Parse the passed string""" diff --git a/python/podio/test_MemberParser.py b/python/podio/test_MemberParser.py index c7e4ec2bb..d99cd11d2 100644 --- a/python/podio/test_MemberParser.py +++ b/python/podio/test_MemberParser.py @@ -243,6 +243,17 @@ def test_parse_valid_no_description(self): self.assertEqual(parsed.description, 'descriptions are not ignored even though they are not required') self.assertTrue(not parsed.is_builtin) + def test_parse_unit(self): + """Test that units are properly parsed""" + parser = MemberParser() + + parsed = parser.parse('unsigned long long var [GeV] // description') + self.assertEqual(parsed.unit, 'GeV') + + parsed = parser.parse('unsigned long long var{42} [GeV] // description') + self.assertEqual(parsed.unit, 'GeV') + self.assertEqual(parsed.default_val, '42') + def test_string_representation(self): """Test that the string representation that is used in the jinja2 templates includes the default initialization""" diff --git a/python/templates/macros/declarations.jinja2 b/python/templates/macros/declarations.jinja2 index 4ee9522f4..af46f499e 100644 --- a/python/templates/macros/declarations.jinja2 +++ b/python/templates/macros/declarations.jinja2 @@ -8,15 +8,15 @@ {% macro member_getters(members, get_syntax) %} {%for member in members %} - /// Access the {{ member.description }} + /// Access the {{ member.docstring }} const {{ member.full_type }}& {{ member.getter_name(get_syntax) }}() const; {% if member.is_array %} - /// Access item i of the {{ member.description }} + /// Access item i of the {{ member.docstring }} const {{ member.array_type }}& {{ member.getter_name(get_syntax) }}(size_t i) const; {%- endif %} {% if member.sub_members %} {% for sub_member in member.sub_members %} - /// Access the member of {{ member.description }} + /// Access the member of {{ member.docstring }} const {{ sub_member.full_type }}& {{ sub_member.getter_name(get_sytnax) }}() const; {% endfor %} {% endif %} @@ -27,24 +27,24 @@ {% macro member_setters(members, get_syntax) %} {% for member in members %} - /// Set the {{ member.description }} + /// Set the {{ member.docstring }} void {{ member.setter_name(get_syntax) }}({{ member.full_type }} value); {% if member.is_array %} void {{ member.setter_name(get_syntax) }}(size_t i, {{ member.array_type }} value); {% endif %} {% if not member.is_builtin %} - /// Get reference to {{ member.description }} + /// Get reference to {{ member.docstring }} {{ member.full_type }}& {{ member.name }}(); {% endif %} {% if member.sub_members %} {% for sub_member in member.sub_members %} {% if sub_member.is_builtin %} - /// Set the member of {{ member.description }} + /// Set the member of {{ member.docstring }} void {{ sub_member.setter_name(get_syntax) }}({{ sub_member.full_type }} value); {% else %} - /// Get reference to the member of {{ member.description }} + /// Get reference to the member of {{ member.docstring }} {{ sub_member.full_type }}& {{ sub_member.name }}(); - /// Set the member of {{ member.description }} + /// Set the member of {{ member.docstring }} void {{ sub_member.setter_name(get_sytnax) }}({{ sub_member.full_type }} value); {% endif %} {% endfor %} diff --git a/tests/datalayout.yaml b/tests/datalayout.yaml index cfca90388..06f8e3075 100755 --- a/tests/datalayout.yaml +++ b/tests/datalayout.yaml @@ -11,10 +11,10 @@ options : components : SimpleStruct: Members: - - int x - - int y - - int z - - std::array p + - int x [mm] + - int y [mm] + - int z [mm] + - std::array p [mm] # can also add c'tors: ExtraCode : declaration: " @@ -26,7 +26,7 @@ components : Description : "A not so simple struct" Author : "Someone" Members: - - SimpleStruct data // component members can have descriptions + - SimpleStruct data [GeV] // component members can have descriptions and units ex2::NamespaceStruct: Members: @@ -66,10 +66,10 @@ datatypes : Author : "B. Hegner" Members: - unsigned long long cellID // cellID - - double x // x-coordinate - - double y // y-coordinate - - double z // z-coordinate - - double energy // measured energy deposit + - double x [mm] // x-coordinate + - double y [mm] // y-coordinate + - double z [mm] // z-coordinate + - double energy [GeV] // measured energy deposit ExampleMC : Description : "Example MC-particle" From 74f39098728e08a12bee451a4ce50ad1908eaad0 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Wed, 30 Aug 2023 19:35:38 +0200 Subject: [PATCH 20/22] Initialize all branches index based for v00-16 series (#471) --- src/ROOTFrameReader.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ROOTFrameReader.cc b/src/ROOTFrameReader.cc index f098ad204..9490a099e 100644 --- a/src/ROOTFrameReader.cc +++ b/src/ROOTFrameReader.cc @@ -155,7 +155,7 @@ void ROOTFrameReader::initCategory(CategoryInfo& catInfo, const std::string& cat // For backwards compatibility make it possible to read the index based files // from older versions - if (m_fileVersion <= podio::version::Version{0, 16, 6}) { + if (m_fileVersion < podio::version::Version{0, 16, 99}) { std::tie(catInfo.branches, catInfo.storedClasses) = createCollectionBranchesIndexBased(catInfo.chain.get(), *catInfo.table, *collInfo); } else { From cda2833249eb6528dfcdd2d9c5c59cb7f7856e33 Mon Sep 17 00:00:00 2001 From: Dmitry Kalinkin Date: Sun, 13 Aug 2023 23:49:20 -0400 Subject: [PATCH 21/22] python/templates: add references between Objects and Collection types --- python/templates/Collection.h.jinja2 | 1 + python/templates/MutableObject.h.jinja2 | 2 ++ python/templates/Object.h.jinja2 | 4 ++++ 3 files changed, 7 insertions(+) diff --git a/python/templates/Collection.h.jinja2 b/python/templates/Collection.h.jinja2 index 24265eef1..cda71791e 100644 --- a/python/templates/Collection.h.jinja2 +++ b/python/templates/Collection.h.jinja2 @@ -48,6 +48,7 @@ A Collection is identified by an ID. */ class {{ class.bare_type }}Collection : public podio::CollectionBase { public: + using value_type = {{ class.bare_type }}; using const_iterator = {{ class.bare_type }}CollectionIterator; using iterator = {{ class.bare_type }}MutableCollectionIterator; diff --git a/python/templates/MutableObject.h.jinja2 b/python/templates/MutableObject.h.jinja2 index ab39778a0..ebdac4f53 100644 --- a/python/templates/MutableObject.h.jinja2 +++ b/python/templates/MutableObject.h.jinja2 @@ -32,6 +32,8 @@ class Mutable{{ class.bare_type }} { friend class {{ class.bare_type }}; public: + using object_type = {{ class.bare_type }}; + using collection_type = {{ class.bare_type }}Collection; {{ macros.constructors_destructors(class.bare_type, Members, prefix='Mutable') }} /// conversion to const object diff --git a/python/templates/Object.h.jinja2 b/python/templates/Object.h.jinja2 index 0a6de9296..42d737cb5 100644 --- a/python/templates/Object.h.jinja2 +++ b/python/templates/Object.h.jinja2 @@ -22,6 +22,7 @@ {{ utils.namespace_open(class.namespace) }} class Mutable{{ class.bare_type }}; +class {{ class.bare_type }}Collection; {{ macros.class_description(class.bare_type, Description, Author) }} class {{ class.bare_type }} { @@ -31,6 +32,9 @@ class {{ class.bare_type }} { friend class {{ class.bare_type }}CollectionIterator; public: + using mutable_type = Mutable{{ class.bare_type }}; + using collection_type = {{ class.bare_type }}Collection; + {{ macros.constructors_destructors(class.bare_type, Members) }} public: From 48a4bf6702c081288853a642749459ac5e42d6d4 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Fri, 8 Sep 2023 12:58:40 +0200 Subject: [PATCH 22/22] Improve the collection interface (#478) * Add empty method to collection interface * Add operator== do collection iterators * Fix clang-tidy warning --- include/podio/CollectionBase.h | 3 +++ include/podio/UserDataCollection.h | 5 +++++ python/templates/Collection.cc.jinja2 | 4 ++++ python/templates/Collection.h.jinja2 | 3 +++ python/templates/macros/iterator.jinja2 | 4 ++++ tests/unittests/frame.cpp | 2 +- tests/unittests/unittest.cpp | 15 ++++++++++----- 7 files changed, 30 insertions(+), 6 deletions(-) diff --git a/include/podio/CollectionBase.h b/include/podio/CollectionBase.h index 6e5b8ded4..ae6fa1d4a 100644 --- a/include/podio/CollectionBase.h +++ b/include/podio/CollectionBase.h @@ -55,6 +55,9 @@ class CollectionBase { /// number of elements in the collection virtual size_t size() const = 0; + /// Is the collection empty + virtual bool empty() const = 0; + /// fully qualified type name virtual const std::string_view getTypeName() const = 0; /// fully qualified type name of elements - with namespace diff --git a/include/podio/UserDataCollection.h b/include/podio/UserDataCollection.h index cc5b7154f..5093816ad 100644 --- a/include/podio/UserDataCollection.h +++ b/include/podio/UserDataCollection.h @@ -136,6 +136,11 @@ class UserDataCollection : public CollectionBase { return _vec.size(); } + /// Is the collection empty + bool empty() const override { + return _vec.empty(); + } + /// fully qualified type name const std::string_view getTypeName() const override { return typeName; diff --git a/python/templates/Collection.cc.jinja2 b/python/templates/Collection.cc.jinja2 index 44b4c139d..b7c49cdab 100644 --- a/python/templates/Collection.cc.jinja2 +++ b/python/templates/Collection.cc.jinja2 @@ -51,6 +51,10 @@ std::size_t {{ collection_type }}::size() const { return m_storage.entries.size(); } +bool {{ collection_type }}::empty() const { + return m_storage.entries.empty(); +} + void {{ collection_type }}::setSubsetCollection(bool setSubset) { if (m_isSubsetColl != setSubset && !m_storage.entries.empty()) { throw std::logic_error("Cannot change the character of a collection that already contains elements"); diff --git a/python/templates/Collection.h.jinja2 b/python/templates/Collection.h.jinja2 index cda71791e..0e49aec53 100644 --- a/python/templates/Collection.h.jinja2 +++ b/python/templates/Collection.h.jinja2 @@ -86,6 +86,9 @@ public: /// number of elements in the collection std::size_t size() const final; + /// Is the collection empty + bool empty() const final; + /// fully qualified type name const std::string_view getTypeName() const final { return typeName; } /// fully qualified type name of elements - with namespace diff --git a/python/templates/macros/iterator.jinja2 b/python/templates/macros/iterator.jinja2 index eaba99ba8..4485504bd 100644 --- a/python/templates/macros/iterator.jinja2 +++ b/python/templates/macros/iterator.jinja2 @@ -11,6 +11,10 @@ public: return m_index != x.m_index; // TODO: may not be complete } + bool operator==(const {{ iterator_type }}& x) const { + return m_index == x.m_index; // TODO: may not be complete + } + {{ prefix }}{{ class.bare_type }} operator*(); {{ prefix }}{{ class.bare_type }}* operator->(); {{ iterator_type }}& operator++(); diff --git a/tests/unittests/frame.cpp b/tests/unittests/frame.cpp index e52965a07..e76803fd7 100644 --- a/tests/unittests/frame.cpp +++ b/tests/unittests/frame.cpp @@ -220,7 +220,7 @@ void checkFrame(const podio::Frame& frame) { REQUIRE(hits[1].energy() == 1); REQUIRE(hits[1].cellID() == 0x123ULL); - REQUIRE(frame.get("emptyClusters").size() == 0); + REQUIRE(frame.get("emptyClusters").empty()); auto& clusters = frame.get("clusters"); REQUIRE(clusters.size() == 2); diff --git a/tests/unittests/unittest.cpp b/tests/unittests/unittest.cpp index cc87410f7..8a313de28 100644 --- a/tests/unittests/unittest.cpp +++ b/tests/unittests/unittest.cpp @@ -80,7 +80,6 @@ TEST_CASE("Assignment-operator ref count", "[basics][memory-management]") { } TEST_CASE("Clearing", "[UBSAN-FAIL][ASAN-FAIL][THREAD-FAIL][basics][memory-management]") { - bool success = true; auto store = podio::EventStore(); auto& hits = store.create("hits"); auto& clusters = store.create("clusters"); @@ -102,10 +101,7 @@ TEST_CASE("Clearing", "[UBSAN-FAIL][ASAN-FAIL][THREAD-FAIL][basics][memory-manag oneRels.push_back(oneRel); } hits.clear(); - if (hits.size() != 0) { - success = false; - } - REQUIRE(success); + REQUIRE(hits.empty()); } TEST_CASE("Cloning", "[basics][memory-management]") { @@ -440,6 +436,15 @@ TEST_CASE("NonPresentCollection", "[basics][event-store]") { REQUIRE_THROWS_AS(store.get("NonPresentCollection"), std::runtime_error); } +TEST_CASE("Collection size and empty", "[basics][collections]") { + ExampleClusterCollection coll{}; + REQUIRE(coll.empty()); + + coll.create(); + coll.create(); + REQUIRE(coll.size() == 2u); +} + TEST_CASE("const correct indexed access to const collections", "[const-correctness]") { STATIC_REQUIRE(std::is_same_v()[0]), ExampleCluster>); // const collections should only have indexed access to mutable