From 8b804068007b9e7bf2bd2f3660a58688e4495019 Mon Sep 17 00:00:00 2001 From: Wanwei Wu Date: Wed, 24 Jun 2026 16:54:28 -0500 Subject: [PATCH 1/2] revert the PR 647 as discussed --- form/form/form_writer.cpp | 25 +- form/form/form_writer.hpp | 8 - form/form_module.cpp | 21 - form/persistence/ipersistence_writer.hpp | 4 +- form/persistence/persistence_writer.cpp | 11 +- form/persistence/persistence_writer.hpp | 5 +- .../root_tbranch_write_container.cpp | 8 - .../root_tbranch_write_container.hpp | 2 - form/root_storage/root_tfile.cpp | 21 +- .../root_ttree_write_container.cpp | 8 - .../root_ttree_write_container.hpp | 1 - form/storage/istorage.hpp | 5 +- form/storage/storage_write_container.cpp | 2 - form/storage/storage_write_container.hpp | 1 - form/storage/storage_writer.cpp | 507 +----------------- form/storage/storage_writer.hpp | 16 +- test/form/form_storage_test.cpp | 360 ------------- test/form/form_test_with_output.jsonnet | 33 -- test/form/writer.cpp | 461 +++------------- 19 files changed, 76 insertions(+), 1423 deletions(-) delete mode 100644 test/form/form_test_with_output.jsonnet diff --git a/form/form/form_writer.cpp b/form/form/form_writer.cpp index cdfbfb651..6a30c82cb 100644 --- a/form/form/form_writer.cpp +++ b/form/form/form_writer.cpp @@ -37,10 +37,7 @@ namespace form::experimental { std::map products = {{pb.label, pb.type}}; m_pers_writer->createContainers(creator, products); - auto declared_it = m_label_to_product_name.find(pb.label); - std::string const& declared_name = - (declared_it != m_label_to_product_name.end()) ? declared_it->second : std::string{}; - m_pers_writer->registerWrite(creator, pb.label, pb.data, *pb.type, declared_name); + m_pers_writer->registerWrite(creator, pb.label, pb.data, *pb.type); m_pers_writer->commitOutput(creator, segment_id); } @@ -69,28 +66,10 @@ namespace form::experimental { for (auto const& pb : products) { // FIXME: We could consider checking id to be identical for all product bases here - auto declared_it = m_label_to_product_name.find(pb.label); - std::string const& declared_name = - (declared_it != m_label_to_product_name.end()) ? declared_it->second : std::string{}; - m_pers_writer->registerWrite(creator, pb.label, pb.data, *pb.type, declared_name); + m_pers_writer->registerWrite(creator, pb.label, pb.data, *pb.type); } m_pers_writer->commitOutput(creator, segment_id); } - void form_writer_interface::declare_product_name(std::string const& routing_label, - std::string const& product_name) - { - m_label_to_product_name[routing_label] = product_name; - } - - void form_writer_interface::finalize() - { - if (m_finalized) { - return; - } - m_finalized = true; - m_pers_writer->finalize(); - } - } diff --git a/form/form/form_writer.hpp b/form/form/form_writer.hpp index 5189710ef..cd6e04e30 100644 --- a/form/form/form_writer.hpp +++ b/form/form/form_writer.hpp @@ -28,17 +28,9 @@ namespace form::experimental { std::string const& segment_id, std::vector const& products); - void finalize(); - - // Explicitly declare the logical ProductName for a given routing label. - // If not called, ProductName defaults to the creator name at write time. - void declare_product_name(std::string const& routing_label, std::string const& product_name); - private: std::unique_ptr m_pers_writer; std::map m_product_to_config; - std::map m_label_to_product_name; - bool m_finalized = false; }; } diff --git a/form/form_module.cpp b/form/form_module.cpp index 05494a126..573fe9864 100644 --- a/form/form_module.cpp +++ b/form/form_module.cpp @@ -45,27 +45,6 @@ namespace { // Initialize FORM interface m_form_interface = std::make_unique(output_cfg, tech_cfg); - - // Explicitly declare ProductNames from user config (products list in jsonnet) - for (auto const& product : products_to_save) { - m_form_interface->declare_product_name(product, product); - } - } - - ~FormOutputModule() - { - if (m_form_interface) { - std::cout << "FormOutputModule destructor: calling finalize() to write metadata\n"; - try { - m_form_interface->finalize(); - std::cout << "FormOutputModule destructor: finalize() completed\n"; - } catch (std::exception const& e) { - std::cerr << "ERROR: FormOutputModule destructor: finalize() failed: " << e.what() - << std::endl; - } catch (...) { - std::cerr << "Unknown error in FormOutputModule destructor during finalize.\n"; - } - } } // This method is called by Phlex - signature must be: void(product_store const&) diff --git a/form/persistence/ipersistence_writer.hpp b/form/persistence/ipersistence_writer.hpp index 89552a4f0..0ab36d28a 100644 --- a/form/persistence/ipersistence_writer.hpp +++ b/form/persistence/ipersistence_writer.hpp @@ -30,10 +30,8 @@ namespace form::detail::experimental { virtual void registerWrite(std::string const& creator, std::string const& label, void const* data, - std::type_info const& type, - std::string const& product_name = "") = 0; + std::type_info const& type) = 0; virtual void commitOutput(std::string const& creator, std::string const& id) = 0; - virtual void finalize() = 0; }; std::unique_ptr createPersistenceWriter(); diff --git a/form/persistence/persistence_writer.cpp b/form/persistence/persistence_writer.cpp index d53a7fc8a..5df720339 100644 --- a/form/persistence/persistence_writer.cpp +++ b/form/persistence/persistence_writer.cpp @@ -52,20 +52,17 @@ void PersistenceWriter::createContainers( void PersistenceWriter::registerWrite(std::string const& creator, std::string const& label, void const* data, - std::type_info const& type, - std::string const& product_name) + std::type_info const& type) { - m_current_creator = creator; // Cache creator for use in commitOutput std::unique_ptr plcmnt = getPlacement(creator, label); - m_store_writer->fillContainer(*plcmnt, data, type, product_name); + m_store_writer->fillContainer(*plcmnt, data, type); return; } void PersistenceWriter::commitOutput(std::string const& creator, std::string const& id) { std::unique_ptr plcmnt = getPlacement(creator, "index"); - // Pass m_current_creator as product_name for IndexRegistry - m_store_writer->fillContainer(*plcmnt, &id, typeid(std::string), m_current_creator); + m_store_writer->fillContainer(*plcmnt, &id, typeid(std::string)); m_store_writer->commitContainers(*plcmnt); return; } @@ -83,5 +80,3 @@ std::unique_ptr PersistenceWriter::getPlacement(std::string const& cr std::string const full_label = buildFullLabel(creator, label); return std::make_unique(config_item->file_name, full_label, config_item->technology); } - -void PersistenceWriter::finalize() { m_store_writer->finalize(m_tech_settings); } diff --git a/form/persistence/persistence_writer.hpp b/form/persistence/persistence_writer.hpp index be1522054..b5f3043cd 100644 --- a/form/persistence/persistence_writer.hpp +++ b/form/persistence/persistence_writer.hpp @@ -34,10 +34,8 @@ namespace form::detail::experimental { void registerWrite(std::string const& creator, std::string const& label, void const* data, - std::type_info const& type, - std::string const& product_name = "") override; + std::type_info const& type) override; void commitOutput(std::string const& creator, std::string const& id) override; - void finalize() override; private: std::unique_ptr getPlacement(std::string const& creator, std::string const& label); @@ -46,7 +44,6 @@ namespace form::detail::experimental { std::unique_ptr m_store_writer; form::experimental::config::ItemConfig m_config_items; form::experimental::config::tech_setting_config m_tech_settings; - std::string m_current_creator; }; } // namespace form::detail::experimental diff --git a/form/root_storage/root_tbranch_write_container.cpp b/form/root_storage/root_tbranch_write_container.cpp index d10eaa3e1..be13fc01e 100644 --- a/form/root_storage/root_tbranch_write_container.cpp +++ b/form/root_storage/root_tbranch_write_container.cpp @@ -130,11 +130,3 @@ void ROOT_TBranch_Write_ContainerImp::commit() m_tree->SetEntries(m_branch->GetEntries()); return; } - -std::uint64_t ROOT_TBranch_Write_ContainerImp::getEntryCount() -{ - if (m_branch == nullptr) { - return 0; - } - return m_branch->GetEntries(); -} diff --git a/form/root_storage/root_tbranch_write_container.hpp b/form/root_storage/root_tbranch_write_container.hpp index a33f7d42d..d61533342 100644 --- a/form/root_storage/root_tbranch_write_container.hpp +++ b/form/root_storage/root_tbranch_write_container.hpp @@ -28,8 +28,6 @@ namespace form::detail::experimental { void fill(void const* data) override; void commit() override; - std::uint64_t getEntryCount() override; - private: std::shared_ptr m_tfile; TTree* m_tree{nullptr}; diff --git a/form/root_storage/root_tfile.cpp b/form/root_storage/root_tfile.cpp index 0a6f8a5a0..5d2beb191 100644 --- a/form/root_storage/root_tfile.cpp +++ b/form/root_storage/root_tfile.cpp @@ -4,31 +4,14 @@ #include "TFile.h" -#include -#include - using namespace form::detail::experimental; ROOT_TFileImp::ROOT_TFileImp(std::string const& name, char mode) : Storage_File(name, mode), m_file(nullptr) { - if (mode == 'c' || mode == 'o') { - // Preserve existing semantics: 'o' recreates the file, matching prior behavior + if (mode == 'c' || mode == 'r' || mode == 'o') { m_file.reset(TFile::Open(name.c_str(), "RECREATE")); - } else if (mode == 'u') { - // 'u' explicitly means reopen/update an existing file while preserving metadata. - if (std::filesystem::exists(name)) { - m_file.reset(TFile::Open(name.c_str(), "UPDATE")); - } else { - m_file.reset(TFile::Open(name.c_str(), "RECREATE")); - } - } else if (mode == 'r' || mode == 'i') { - m_file.reset(TFile::Open(name.c_str(), "READ")); } else { - throw std::runtime_error(std::string("Unsupported ROOT file open mode: ") + mode); - } - - if (!m_file || m_file->IsZombie()) { - throw std::runtime_error("Failed to open ROOT file: " + name); + m_file.reset(TFile::Open(name.c_str(), "READ")); } } diff --git a/form/root_storage/root_ttree_write_container.cpp b/form/root_storage/root_ttree_write_container.cpp index 4f88c496c..5ad222673 100644 --- a/form/root_storage/root_ttree_write_container.cpp +++ b/form/root_storage/root_ttree_write_container.cpp @@ -57,14 +57,6 @@ void ROOT_TTree_Write_ContainerImp::commit() TTree* ROOT_TTree_Write_ContainerImp::getTTree() { return m_tree.get(); } -std::uint64_t ROOT_TTree_Write_ContainerImp::getEntryCount() -{ - if (m_tree == nullptr) { - return 0; - } - return m_tree->GetEntries(); -} - void ROOT_TTree_Write_ContainerImp::TTreeDeleter::operator()(gsl::owner t) const { if (t) { diff --git a/form/root_storage/root_ttree_write_container.hpp b/form/root_storage/root_ttree_write_container.hpp index 38edfa2e0..4e20f1a7e 100644 --- a/form/root_storage/root_ttree_write_container.hpp +++ b/form/root_storage/root_ttree_write_container.hpp @@ -28,7 +28,6 @@ namespace form::detail::experimental { void commit() override; TTree* getTTree(); - std::uint64_t getEntryCount() override; private: // Be absolutely explicit about the ownership semantics of TTree*, diff --git a/form/storage/istorage.hpp b/form/storage/istorage.hpp index 10c73ecd0..555d58b2b 100644 --- a/form/storage/istorage.hpp +++ b/form/storage/istorage.hpp @@ -38,10 +38,8 @@ namespace form::detail::experimental { form::experimental::config::tech_setting_config const& settings) = 0; virtual void fillContainer(Placement const& plcmnt, void const* data, - std::type_info const& type, - std::string const& product_name = "") = 0; + std::type_info const& type) = 0; virtual void commitContainers(Placement const& plcmnt) = 0; - virtual void finalize(form::experimental::config::tech_setting_config const& settings) = 0; }; class IStorage_File { @@ -68,7 +66,6 @@ namespace form::detail::experimental { virtual void commit() = 0; virtual void setAttribute(std::string const& name, std::string const& value) = 0; - virtual std::uint64_t getEntryCount() = 0; }; class IStorage_Read_Container { diff --git a/form/storage/storage_write_container.cpp b/form/storage/storage_write_container.cpp index 5aaceda6e..37742aca3 100644 --- a/form/storage/storage_write_container.cpp +++ b/form/storage/storage_write_container.cpp @@ -27,5 +27,3 @@ void Storage_Write_Container::setAttribute(std::string const& /*name*/, "Storage_Write_Container::setAttribute does not accept any attributes for a container named " + m_name); } - -std::uint64_t Storage_Write_Container::getEntryCount() { return 0; } diff --git a/form/storage/storage_write_container.hpp b/form/storage/storage_write_container.hpp index dd8ce4ab5..198604357 100644 --- a/form/storage/storage_write_container.hpp +++ b/form/storage/storage_write_container.hpp @@ -24,7 +24,6 @@ namespace form::detail::experimental { void commit() override; void setAttribute(std::string const& name, std::string const& value) override; - std::uint64_t getEntryCount() override; private: std::string m_name; diff --git a/form/storage/storage_writer.cpp b/form/storage/storage_writer.cpp index 7a1c9f819..a4a9194cf 100644 --- a/form/storage/storage_writer.cpp +++ b/form/storage/storage_writer.cpp @@ -5,238 +5,10 @@ #include "storage_file.hpp" #include "storage_write_association.hpp" -#include "form/technology.hpp" #include "util/factories.hpp" -#include -#include -#include -#include -#include -#include -#include - -#ifdef USE_ROOT_STORAGE -#include "TFile.h" -#include "TObjString.h" -#include "TTree.h" -#include "root_storage/root_tfile.hpp" -#include -#endif - using namespace form::detail::experimental; -namespace { - form::experimental::config::tech_setting_config::table_t lookup_file_table( - form::experimental::config::tech_setting_config const& settings, - int technology, - std::string const& file_name) - { - auto const per_tech_it = settings.file_settings.find(technology); - if (per_tech_it == settings.file_settings.end()) { - return {}; - } - auto const file_it = per_tech_it->second.find(file_name); - if (file_it == per_tech_it->second.end()) { - return {}; - } - return file_it->second; - } - - form::experimental::config::tech_setting_config::table_t lookup_container_table( - form::experimental::config::tech_setting_config const& settings, - int technology, - std::string const& container_name) - { - auto const per_tech_it = settings.container_settings.find(technology); - if (per_tech_it == settings.container_settings.end()) { - return {}; - } - auto const container_it = per_tech_it->second.find(container_name); - if (container_it == per_tech_it->second.end()) { - return {}; - } - return container_it->second; - } - - std::string trim_copy(std::string_view input) - { - auto is_space = [](unsigned char c) { return std::isspace(c) != 0; }; - while (!input.empty() && is_space(static_cast(input.front()))) { - input.remove_prefix(1); - } - while (!input.empty() && is_space(static_cast(input.back()))) { - input.remove_suffix(1); - } - return std::string{input}; - } - - bool parse_uint64(std::string const& value_text, std::uint64_t& value) - { - if (value_text.empty()) { - return false; - } - - int base = 10; - if (value_text.size() > 2 && value_text[0] == '0' && - (value_text[1] == 'x' || value_text[1] == 'X')) { - base = 16; - } else { - bool has_hex_letter = false; - for (char ch : value_text) { - if ((ch >= 'a' && ch <= 'f') || (ch >= 'A' && ch <= 'F')) { - has_hex_letter = true; - break; - } - } - if (has_hex_letter) { - base = 16; - } - } - - auto const* begin = value_text.data(); - auto const* end = value_text.data() + value_text.size(); - auto [ptr, ec] = std::from_chars(begin, end, value, base); - return ec == std::errc{} && ptr == end; - } - - bool parse_layer_index_string(std::string const& index_text, - std::vector& schema, - std::vector& values) - { - schema.clear(); - values.clear(); - - auto content = trim_copy(index_text); - if (content.size() >= 2 && content.front() == '[' && content.back() == ']') { - content = content.substr(1, content.size() - 2); - } - - std::size_t start = 0; - while (start < content.size()) { - auto end = content.find_first_of(",;", start); - auto token = trim_copy( - content.substr(start, end == std::string::npos ? std::string::npos : end - start)); - if (!token.empty()) { - auto sep_pos = token.find(':'); - if (sep_pos == std::string::npos) { - sep_pos = token.find('='); - } - if (sep_pos == std::string::npos) { - return false; - } - - auto layer_name = trim_copy(token.substr(0, sep_pos)); - auto value_text = trim_copy(token.substr(sep_pos + 1)); - if (layer_name.empty()) { - return false; - } - - std::uint64_t value = 0; - if (!parse_uint64(value_text, value)) { - return false; - } - - schema.push_back(std::move(layer_name)); - values.push_back(value); - } - - if (end == std::string::npos) { - break; - } - start = end + 1; - } - - return !schema.empty() && schema.size() == values.size(); - } - - bool is_index_container(std::string const& container_name) - { - return container_name.size() >= 6 && - container_name.compare(container_name.size() - 6, 6, "/index") == 0; - } - - std::pair split_creator_and_product(std::string const& container_name) - { - std::size_t const slash = container_name.find('/'); - if (slash == std::string::npos) { - return {std::string{}, container_name}; - } - - std::string creator = container_name.substr(0, slash); - std::string product = container_name.substr(slash + 1); - return {std::move(creator), std::move(product)}; - } - - bool setting_key_is_process_name(std::string const& key) - { - std::string lowered; - lowered.reserve(key.size()); - for (char c : key) { - lowered.push_back(static_cast(std::tolower(static_cast(c)))); - } - return lowered == "processname" || lowered == "process_name"; - } - - std::string resolve_process_name(form::experimental::config::tech_setting_config const& settings, - std::string const& file_name) - { - for (auto const& [technology, by_file] : settings.file_settings) { - (void)technology; - auto const file_it = by_file.find(file_name); - if (file_it == by_file.end()) { - continue; - } - for (auto const& [key, value] : file_it->second) { - if (setting_key_is_process_name(key) && !value.empty()) { - return value; - } - } - } - return std::string(); - } - - std::string build_product_id(std::string const& product_name, - std::string const& producer, - std::string const& process_name) - { - std::string product_id; - product_id.reserve(product_name.size() + producer.size() + process_name.size() + 2); - product_id += product_name; - product_id += '|'; - product_id += producer; - product_id += '|'; - product_id += process_name; - return product_id; - } - - std::string serialize_layer_schema(std::vector const& schema) - { - std::ostringstream oss; - oss << '['; - for (std::size_t i = 0; i < schema.size(); ++i) { - if (i > 0) { - oss << ','; - } - oss << '"' << schema[i] << '"'; - } - oss << ']'; - return oss.str(); - } - - std::string schema_key_for_match(std::vector const& schema) - { - std::ostringstream oss; - for (std::size_t i = 0; i < schema.size(); ++i) { - if (i > 0) { - oss << ','; - } - oss << schema[i]; - } - return oss.str(); - } -} - // Factory function implementation namespace form::detail::experimental { std::unique_ptr createStorageWriter() @@ -262,7 +34,7 @@ void StorageWriter::createContainers( .insert({plcmnt->fileName(), createFile(plcmnt->technology(), plcmnt->fileName(), 'o')}) .first; for (auto const& [key, value] : - lookup_file_table(settings, plcmnt->technology(), plcmnt->fileName())) + settings.getFileTable(plcmnt->technology(), plcmnt->fileName())) file->second->setAttribute(key, value); } // Create and bind container to file @@ -287,7 +59,7 @@ void StorageWriter::createContainers( } for (auto const& [key, value] : - lookup_container_table(settings, plcmnt->technology(), plcmnt->containerName())) + settings.getContainerTable(plcmnt->technology(), plcmnt->containerName())) container->setAttribute(key, value); container->setFile(file->second); container->setupWrite(*type); @@ -298,8 +70,7 @@ void StorageWriter::createContainers( void StorageWriter::fillContainer(Placement const& plcmnt, void const* data, - std::type_info const& /* type*/, - std::string const& product_name) + std::type_info const& /* type*/) { // Use file+container as composite key auto key = std::make_pair(plcmnt.fileName(), plcmnt.containerName()); @@ -309,77 +80,7 @@ void StorageWriter::fillContainer(Placement const& plcmnt, throw std::runtime_error("StorageWriter::fillContainer Container doesn't exist: " + plcmnt.containerName()); } - - auto const [creator_name, product_from_container] = - split_creator_and_product(plcmnt.containerName()); - (void)product_from_container; - - if (!is_index_container(plcmnt.containerName())) { - if (!creator_name.empty()) { - std::string const logical_product_name = !product_name.empty() ? product_name : creator_name; - m_productsByProducer[plcmnt.fileName()][creator_name].insert(logical_product_name); - auto& pending_products = m_pendingProductsByProducer[plcmnt.fileName()][creator_name]; - if (std::find(pending_products.begin(), pending_products.end(), logical_product_name) == - pending_products.end()) { - pending_products.push_back(logical_product_name); - } - } - } - - std::uint64_t payload_row = 0; - if (is_index_container(plcmnt.containerName())) { - // Keep payload row 0-based so it maps directly to entry/row ids. - payload_row = cont->second->getEntryCount(); - } - - if (is_index_container(plcmnt.containerName()) && data != nullptr) { - auto const* segment_id = static_cast(data); - std::vector schema; - std::vector values; - if (parse_layer_index_string(*segment_id, schema, values)) { - auto& pending_by_producer = m_pendingProductsByProducer[plcmnt.fileName()]; - auto pending_it = pending_by_producer.find(creator_name); - - std::vector products_for_index; - if (pending_it != pending_by_producer.end()) { - products_for_index = std::move(pending_it->second); - pending_by_producer.erase(pending_it); - } - - if (products_for_index.empty() && !creator_name.empty()) { - auto const all_by_producer = m_productsByProducer.find(plcmnt.fileName()); - if (all_by_producer != m_productsByProducer.end()) { - auto const all_products_it = all_by_producer->second.find(creator_name); - if (all_products_it != all_by_producer->second.end()) { - products_for_index.assign(all_products_it->second.begin(), - all_products_it->second.end()); - } - } - } - - if (products_for_index.empty()) { - m_indexLayerSchemas[plcmnt.fileName()].push_back(std::move(schema)); - m_indexLayerValues[plcmnt.fileName()].push_back(std::move(values)); - m_indexProductNames[plcmnt.fileName()].push_back(std::string()); - m_indexProducers[plcmnt.fileName()].push_back(std::string()); - m_indexContainerNames[plcmnt.fileName()].push_back(std::string()); - m_indexPayloadRows[plcmnt.fileName()].push_back(payload_row); - } else { - for (auto const& pending_product : products_for_index) { - m_indexLayerSchemas[plcmnt.fileName()].push_back(schema); - m_indexLayerValues[plcmnt.fileName()].push_back(values); - m_indexProductNames[plcmnt.fileName()].push_back(pending_product); - m_indexProducers[plcmnt.fileName()].push_back(creator_name); - m_indexContainerNames[plcmnt.fileName()].push_back(creator_name); - m_indexPayloadRows[plcmnt.fileName()].push_back(payload_row); - } - } - } - } - - if (data != nullptr) - cont->second->fill(data); - //TODO: decide whether to signal phlex with e.g. an exception if we get nullptr here + cont->second->fill(data); return; } @@ -390,203 +91,3 @@ void StorageWriter::commitContainers(Placement const& plcmnt) cont->second->commit(); return; } - -static std::string generateUUID() -{ -#ifdef USE_ROOT_STORAGE - TUUID uuid; - return uuid.AsString(); -#else -#error "ROOT storage is disabled, please provide an alternative UUID generator." -#endif -} - -void StorageWriter::finalize(form::experimental::config::tech_setting_config const& settings) -{ - // Write FileCatalog metadata tree for each file -#ifdef USE_ROOT_STORAGE - for (auto const& [fileName, file] : m_files) { - // Try to downcast to ROOT file implementation - ROOT_TFileImp* root_file = dynamic_cast(file.get()); - if (root_file == nullptr) { - continue; - } - - auto tfile = root_file->getTFile(); - if (!tfile) { - continue; - } - - // Preserve any existing FileCatalog metadata if reopening the file. - TObject* existing_catalog_obj = tfile->Get("FileCatalog"); - TTree* existing_catalog = dynamic_cast(existing_catalog_obj); - if (existing_catalog != nullptr) { - // Existing FileCatalog already contains the persisted FileUUID. - continue; - } - - // Create FileCatalog tree for new files. - auto catalog = std::make_unique("FileCatalog", "File-level metadata catalog"); - catalog->SetDirectory(nullptr); - - // Determine FileFormatVersion based on technology - // We need to get technology from somewhere - for now, infer from existing containers - int fileFormatVersion = 1; // Default: ROOT TTree - - // Check if any container in this file is ROOT_RNTUPLE - for (auto const& [key, container] : m_write_containers) { - if (key.first == fileName) { - // We could check technology here, but for now assume ROOT_TTREE (version 1) - // In future, this could be enhanced to detect ROOT_RNTUPLE (version 2) - break; - } - } - - std::string fileUUID = generateUUID(); - catalog->Branch("FileUUID", &fileUUID); - // Add FileFormatVersion branch - catalog->Branch("FileFormatVersion", &fileFormatVersion, "FileFormatVersion/I"); - - // Fill the tree with one entry - catalog->Fill(); - - // Write to file - tfile->WriteTObject(catalog.get()); - - // Create ProductRegistry tree listing logical product names from user/framework config. - // Use explicit logical product names instead of deriving from container structure. - std::string const user_provided_process_name = resolve_process_name(settings, fileName); - - auto products_it = m_productsByProducer.find(fileName); - if (products_it != m_productsByProducer.end() && !products_it->second.empty()) { - auto const& products_by_producer = products_it->second; - auto registry = std::make_unique("ProductRegistry", "Product-level metadata catalog"); - registry->SetDirectory(nullptr); - - std::string productName; - std::string processName; - std::string producer; - std::string productID; - - registry->Branch("ProductName", &productName); - registry->Branch("ProcessName", &processName); - registry->Branch("Producer", &producer); - registry->Branch("ProductID", &productID); - - for (auto const& [creator, product_names] : products_by_producer) { - for (auto const& nm : product_names) { - productName = nm; - processName = user_provided_process_name; - producer = creator; - productID = build_product_id(productName, producer, processName); - registry->Fill(); - } - } - tfile->WriteTObject(registry.get()); - } - - auto schema_it = m_indexLayerSchemas.find(fileName); - auto values_it = m_indexLayerValues.find(fileName); - auto idx_products_it = m_indexProductNames.find(fileName); - auto idx_producers_it = m_indexProducers.find(fileName); - auto containers_it = m_indexContainerNames.find(fileName); - auto payload_rows_it = m_indexPayloadRows.find(fileName); - if (schema_it != m_indexLayerSchemas.end() && values_it != m_indexLayerValues.end() && - idx_products_it != m_indexProductNames.end() && - idx_producers_it != m_indexProducers.end() && - containers_it != m_indexContainerNames.end() && - payload_rows_it != m_indexPayloadRows.end()) { - auto const& schemas = schema_it->second; - auto const& values = values_it->second; - auto const& product_names = idx_products_it->second; - auto const& producers = idx_producers_it->second; - auto const& container_names = containers_it->second; - auto const& payload_rows = payload_rows_it->second; - if (!schemas.empty() && schemas.size() == values.size() && - schemas.size() == product_names.size() && schemas.size() == producers.size() && - schemas.size() == container_names.size() && schemas.size() == payload_rows.size()) { - std::string canonical_schema_key; - std::size_t canonical_schema_size = 0; - std::size_t canonical_schema_count = 0; - std::map> schema_stats; - for (auto const& schema : schemas) { - std::string schema_key = schema_key_for_match(schema); - auto& [count, size] = schema_stats[schema_key]; - ++count; - size = schema.size(); - if (count > canonical_schema_count || - (count == canonical_schema_count && size > canonical_schema_size)) { - canonical_schema_count = count; - canonical_schema_size = size; - canonical_schema_key = schema_key; - } - } - - if (canonical_schema_key.empty()) { - continue; - } - - auto index_registry = - std::make_unique("IndexRegistry", "Layer index metadata catalog"); - index_registry->SetDirectory(nullptr); - - std::vector canonical_schema; - { - std::size_t start = 0; - while (start <= canonical_schema_key.size()) { - std::size_t end = canonical_schema_key.find(',', start); - std::string token = canonical_schema_key.substr( - start, end == std::string::npos ? std::string::npos : end - start); - if (!token.empty()) { - canonical_schema.push_back(token); - } - if (end == std::string::npos) { - break; - } - start = end + 1; - } - } - - std::vector layer_values_by_branch(canonical_schema.size(), 0); - for (std::size_t i = 0; i < canonical_schema.size(); ++i) { - std::string const leaf_list = canonical_schema[i] + "/l"; - index_registry->Branch( - canonical_schema[i].c_str(), &layer_values_by_branch[i], leaf_list.c_str()); - } - std::string index_product_id; - index_registry->Branch("ProductID", &index_product_id); - std::string index_container_name; - index_registry->Branch("ContainerName", &index_container_name); - std::uint64_t index_payload_row = 0; - index_registry->Branch("PayloadRow", &index_payload_row, "PayloadRow/l"); - - // Keep schema in tree header metadata as ["layer1","layer2",...]. - std::string schema_header = serialize_layer_schema(canonical_schema); - TList* userInfo = index_registry->GetUserInfo(); - if (userInfo) { - userInfo->SetOwner(kTRUE); - // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) - userInfo->Add(new TObjString(schema_header.c_str())); - } - - for (std::size_t i = 0; i < schemas.size(); ++i) { - if (schema_key_for_match(schemas[i]) != canonical_schema_key) { - continue; - } - for (std::size_t layer_idx = 0; layer_idx < canonical_schema.size(); ++layer_idx) { - layer_values_by_branch[layer_idx] = values[i][layer_idx]; - } - index_product_id = - build_product_id(product_names[i], producers[i], user_provided_process_name); - index_container_name = container_names[i]; - index_payload_row = payload_rows[i]; - index_registry->Fill(); - } - - tfile->WriteTObject(index_registry.get()); - } - } - } -#endif - return; -} diff --git a/form/storage/storage_writer.hpp b/form/storage/storage_writer.hpp index 1f8dcc2bf..50679d317 100644 --- a/form/storage/storage_writer.hpp +++ b/form/storage/storage_writer.hpp @@ -6,14 +6,11 @@ #include "istorage.hpp" #include "storage_utils.hpp" -#include #include #include -#include #include #include #include // for std::pair -#include namespace form::detail::experimental { @@ -28,10 +25,8 @@ namespace form::detail::experimental { form::experimental::config::tech_setting_config const& settings) override; void fillContainer(Placement const& plcmnt, void const* data, - std::type_info const& type, - std::string const& product_name = "") override; + std::type_info const& type) override; void commitContainers(Placement const& plcmnt) override; - void finalize(form::experimental::config::tech_setting_config const& settings) override; private: std::map> m_files; @@ -40,15 +35,6 @@ namespace form::detail::experimental { pair_hash> m_write_containers; std::map> m_indexMaps; - std::map>> m_indexLayerSchemas; - std::map>> m_indexLayerValues; - std::map> m_indexProductNames; - std::map> m_indexProducers; - std::map> m_indexContainerNames; - std::map> m_indexPayloadRows; - std::map>> m_productsByProducer; - std::map>> - m_pendingProductsByProducer; }; } // namespace form::detail::experimental diff --git a/test/form/form_storage_test.cpp b/test/form/form_storage_test.cpp index c1a046f82..0cf32e305 100644 --- a/test/form/form_storage_test.cpp +++ b/test/form/form_storage_test.cpp @@ -2,14 +2,6 @@ #include "test/form/test_utils.hpp" -#include "core/placement.hpp" -#include "form/config.hpp" -#include "form/technology.hpp" -#include "root_storage/root_tfile.hpp" -#include "storage/storage_file.hpp" -#include "storage/storage_write_container.hpp" -#include "storage/storage_writer.hpp" - #include "TFile.h" #include "TTree.h" @@ -17,7 +9,6 @@ #include #include -#include #include using namespace form::detail::experimental; @@ -278,354 +269,3 @@ TEST_CASE("Root TTree write container: fill and commit are not implemented", "[f CHECK_THROWS_AS(writeAssoc->fill(dummy), std::runtime_error); CHECK_THROWS_AS(writeAssoc->commit(), std::runtime_error); } - -TEST_CASE("Root file open modes and attribute validation", "[form]") -{ - std::string const file_name = "testRootTFileModes.root"; - - { - auto seed = TFile::Open(file_name.c_str(), "RECREATE"); - REQUIRE(seed != nullptr); - seed->Write(); - seed->Close(); - } - - SECTION("update mode reopens existing file") - { - ROOT_TFileImp file(file_name, 'u'); - REQUIRE(file.getTFile() != nullptr); - } - - SECTION("update mode creates file when missing") - { - std::string const missing_name = "testRootTFileModesMissing.root"; - ROOT_TFileImp file(missing_name, 'u'); - REQUIRE(file.getTFile() != nullptr); - } - - SECTION("read mode opens file") - { - ROOT_TFileImp file(file_name, 'r'); - REQUIRE(file.getTFile() != nullptr); - } - - SECTION("unknown compression token keeps ROOT default") - { - ROOT_TFileImp file(file_name, 'o'); - int default_level = file.getTFile()->GetCompressionLevel(); - CHECK_NOTHROW(file.setAttribute("compression", "NotARealROOTCompressionAlgo")); - CHECK(file.getTFile()->GetCompressionLevel() == default_level); - } - - SECTION("unsupported attribute throws") - { - ROOT_TFileImp file(file_name, 'o'); - CHECK_THROWS_AS(file.setAttribute("does_not_exist", "1"), std::runtime_error); - } - - SECTION("unsupported file mode throws") - { - CHECK_THROWS_AS(ROOT_TFileImp(file_name, 'z'), std::runtime_error); - } -} - -TEST_CASE("Storage write container error paths", "[form]") -{ - auto container = createWriteContainer(technology, "test/testTree"); - REQUIRE(container != nullptr); - - CHECK(container->getEntryCount() == 0); - CHECK_THROWS_AS(container->setupWrite(typeid(int)), std::runtime_error); - - std::shared_ptr wrong_file(new Storage_File("container_error.root", 'o')); - - CHECK_THROWS_AS(container->setFile(wrong_file), std::runtime_error); -} - -TEST_CASE("Storage write container default attribute rejection", "[form]") -{ - Storage_Write_Container container("test/container"); - CHECK_THROWS_AS(container.setAttribute("auto_flush", "1"), std::runtime_error); -} - -TEST_CASE("Storage write container rejects unknown attributes", "[form]") -{ - auto container = createWriteContainer(technology, "test/branch"); - REQUIRE(container != nullptr); - - CHECK_THROWS_AS(container->setAttribute("not_supported", "1"), std::runtime_error); -} - -TEST_CASE("StorageWriter finalize skips IndexRegistry for unparsable index", "[form]") -{ - std::string const file_name = "testStorageWriterInvalidIndex.root"; - StorageWriter writer; - form::experimental::config::tech_setting_config settings; - - std::map, std::type_info const*> containers; - containers.emplace(std::make_unique(file_name, "UnitTest/value", technology), - &typeid(std::vector)); - containers.emplace(std::make_unique(file_name, "UnitTest/index", technology), - &typeid(std::string)); - writer.createContainers(containers, settings); - - Placement payload_placement(file_name, "UnitTest/value", technology); - std::vector payload = {1, 2, 3}; - writer.fillContainer(payload_placement, &payload, typeid(std::vector), "DeclaredProduct"); - - Placement index_placement(file_name, "UnitTest/index", technology); - std::string bad_index = "EVENT0001"; - writer.fillContainer(index_placement, &bad_index, typeid(std::string), "UnitTest"); - writer.commitContainers(index_placement); - writer.finalize(settings); - - auto file = TFile::Open(file_name.c_str(), "READ"); - REQUIRE(file != nullptr); - REQUIRE_FALSE(file->IsZombie()); - - CHECK(file->Get("FileCatalog") != nullptr); - CHECK(file->Get("ProductRegistry") != nullptr); - CHECK(file->Get("IndexRegistry") == nullptr); - - file->Close(); -} - -TEST_CASE("Storage_Write_Container base class exercises all default methods", "[form]") -{ - Storage_Write_Container c("my/container"); - - // name() and getEntryCount() on base class - CHECK(c.name() == "my/container"); - CHECK(c.getEntryCount() == 0u); - - // setFile, setupWrite, fill, commit are all no-ops on the base class - auto dummy_file = std::make_shared("dummy_base_test.root", 'o'); - CHECK_NOTHROW(c.setFile(dummy_file)); - CHECK_NOTHROW(c.setupWrite(typeid(int))); - int val = 99; - CHECK_NOTHROW(c.fill(&val)); - CHECK_NOTHROW(c.commit()); -} - -TEST_CASE("StorageWriter fillContainer throws when container is not registered", "[form]") -{ - StorageWriter writer; - form::experimental::config::tech_setting_config settings; - // No createContainers called, so m_write_containers is empty - Placement p("ghost.root", "NoSuch/container", technology); - int d = 1; - CHECK_THROWS_AS(writer.fillContainer(p, &d, typeid(int), ""), std::runtime_error); -} - -TEST_CASE("StorageWriter fillContainer with empty product_name falls back to creator_name", - "[form]") -{ - std::string const file_name = "testEmptyProductName.root"; - StorageWriter writer; - form::experimental::config::tech_setting_config settings; - - std::map, std::type_info const*> containers; - containers.emplace(std::make_unique(file_name, "Creator/value", technology), - &typeid(std::vector)); - containers.emplace(std::make_unique(file_name, "Creator/index", technology), - &typeid(std::string)); - writer.createContainers(containers, settings); - - // Empty product_name → logical_product_name falls back to creator_name ("Creator") - Placement pp(file_name, "Creator/value", technology); - std::vector d = {7}; - CHECK_NOTHROW(writer.fillContainer(pp, &d, typeid(std::vector), "")); - - Placement ip(file_name, "Creator/index", technology); - std::string idx = "[RUN=00000001;EVT=00000002]"; - writer.fillContainer(ip, &idx, typeid(std::string), "Creator"); - writer.commitContainers(ip); - writer.finalize(settings); - - auto f = TFile::Open(file_name.c_str(), "READ"); - REQUIRE(f != nullptr); - TTree* reg = f->Get("ProductRegistry"); - REQUIRE(reg != nullptr); - std::string* prod_name = nullptr; - reg->SetBranchAddress("ProductName", &prod_name); - reg->GetEntry(0); - REQUIRE(prod_name != nullptr); - // product_name was empty, so creator_name ("Creator") was used - CHECK(*prod_name == "Creator"); - f->Close(); -} - -TEST_CASE("StorageWriter createContainers reuses existing parent TTree", "[form]") -{ - std::string const file_name = "testSharedParentTTree.root"; - StorageWriter writer; - form::experimental::config::tech_setting_config settings; - - // First call: creates "SharedTree" parent + "branch1" container - { - std::map, std::type_info const*> c1; - c1.emplace(std::make_unique(file_name, "SharedTree/branch1", technology), - &typeid(std::vector)); - writer.createContainers(c1, settings); - } - - // Second call: "SharedTree" parent already in m_write_containers → hits else branch - { - std::map, std::type_info const*> c2; - c2.emplace(std::make_unique(file_name, "SharedTree/branch2", technology), - &typeid(float)); - CHECK_NOTHROW(writer.createContainers(c2, settings)); - } - - // Both branches must be usable without errors - std::vector d1 = {1, 2}; - float d2 = 2.71f; - CHECK_NOTHROW(writer.fillContainer( - Placement(file_name, "SharedTree/branch1", technology), &d1, typeid(std::vector), "p1")); - CHECK_NOTHROW(writer.fillContainer( - Placement(file_name, "SharedTree/branch2", technology), &d2, typeid(float), "p2")); - CHECK_NOTHROW(writer.finalize(settings)); -} - -TEST_CASE("StorageWriter fillContainer index with nullptr data skips index recording", "[form]") -{ - std::string const file_name = "testNullIndexData.root"; - StorageWriter writer; - form::experimental::config::tech_setting_config settings; - - std::map, std::type_info const*> containers; - containers.emplace(std::make_unique(file_name, "Prod/index", technology), - &typeid(std::string)); - writer.createContainers(containers, settings); - - Placement ip(file_name, "Prod/index", technology); - // data == nullptr → is_index_container && data != nullptr evaluates false → inner block skipped - CHECK_NOTHROW(writer.fillContainer(ip, nullptr, typeid(std::string), "Prod")); - writer.finalize(settings); - - auto f = TFile::Open(file_name.c_str(), "READ"); - REQUIRE(f != nullptr); - CHECK(f->Get("FileCatalog") != nullptr); - // No parsable index recorded → no IndexRegistry - CHECK(f->Get("IndexRegistry") == nullptr); - f->Close(); -} - -TEST_CASE( - "StorageWriter finalize with no payload: no ProductRegistry; index entry with empty fields", - "[form]") -{ - std::string const file_name = "testNoPayloadProducts.root"; - StorageWriter writer; - form::experimental::config::tech_setting_config settings; - - // Register only an index container — no payload container - std::map, std::type_info const*> containers; - containers.emplace(std::make_unique(file_name, "Anon/index", technology), - &typeid(std::string)); - writer.createContainers(containers, settings); - - // Fill index with valid parsable data; no payload registered for "Anon" - // → products_for_index empty + m_productsByProducer empty → push empty strings into index - Placement ip(file_name, "Anon/index", technology); - std::string idx = "[RUN=00000001]"; - writer.fillContainer(ip, &idx, typeid(std::string), "Anon"); - writer.commitContainers(ip); - writer.finalize(settings); - - auto f = TFile::Open(file_name.c_str(), "READ"); - REQUIRE(f != nullptr); - CHECK(f->Get("FileCatalog") != nullptr); - // m_productsByProducer empty → no ProductRegistry - CHECK(f->Get("ProductRegistry") == nullptr); - // Index data was recorded (with empty product info) → IndexRegistry is written - CHECK(f->Get("IndexRegistry") != nullptr); - f->Close(); -} - -TEST_CASE("Root TTree setupWrite finds pre-existing TTree; getEntryCount reflects entries", - "[form]") -{ - std::string const file_name = "testPreExistingTTree.root"; - - // Step 1: create a file with a TTree that has some entries using plain ROOT - { - auto* seed_file = TFile::Open(file_name.c_str(), "RECREATE"); - REQUIRE(seed_file != nullptr); - auto* tree = new TTree("ExistingTree", "seed"); - int x = 0; - tree->Branch("x", &x, "x/I"); - for (x = 0; x < 3; ++x) { - tree->Fill(); - } - seed_file->Write(); - seed_file->Close(); - } - - // Step 2: open in update mode; setupWrite must find the existing TTree - auto root_file = std::make_shared(file_name, 'u'); - ROOT_TTree_Write_ContainerImp container( - "ExistingTree"); //TODO: use FORM factory functions instead of explicit TTree - container.setFile(root_file); - // setupWrite: first m_tree.reset(Get()) returns non-null → skips new-tree branch - CHECK_NOTHROW(container.setupWrite(typeid(void))); - CHECK(container.getTTree() != nullptr); - // getEntryCount() where m_tree != nullptr returns m_tree->GetEntries() - CHECK(container.getEntryCount() == 3u); -} - -TEST_CASE("StorageWriter parses colon indices and honors process_name key", "[form]") -{ - std::string const file_name = "testStorageWriterColonIndex.root"; - StorageWriter writer; - form::experimental::config::tech_setting_config settings; - - std::map, std::type_info const*> containers; - containers.emplace(std::make_unique(file_name, "UnitTest/value", technology), - &typeid(std::vector)); - containers.emplace(std::make_unique(file_name, "UnitTest/index", technology), - &typeid(std::string)); - writer.createContainers(containers, settings); - - Placement payload_placement(file_name, "UnitTest/value", technology); - std::vector payload = {10, 20, 30}; - writer.fillContainer(payload_placement, &payload, typeid(std::vector), "DeclaredProduct"); - - Placement index_placement(file_name, "UnitTest/index", technology); - std::string index_text = "[EVENT:0000000A;SEG:0000000B]"; - writer.fillContainer(index_placement, &index_text, typeid(std::string), "UnitTest"); - writer.commitContainers(index_placement); - writer.finalize(settings); - - auto file = TFile::Open(file_name.c_str(), "READ"); - REQUIRE(file != nullptr); - REQUIRE_FALSE(file->IsZombie()); - - TTree* registry = file->Get("ProductRegistry"); - REQUIRE(registry != nullptr); - REQUIRE(registry->GetEntries() > 0); - std::string* process_name = nullptr; - registry->SetBranchAddress("ProcessName", &process_name); - registry->GetEntry(0); - REQUIRE(process_name != nullptr); - CHECK(process_name->empty()); - - TTree* index_registry = file->Get("IndexRegistry"); - REQUIRE(index_registry != nullptr); - REQUIRE(index_registry->GetEntries() > 0); - - unsigned long long event_value = 0; - unsigned long long seg_value = 0; - std::string* product_id = nullptr; - index_registry->SetBranchAddress("EVENT", &event_value); - index_registry->SetBranchAddress("SEG", &seg_value); - index_registry->SetBranchAddress("ProductID", &product_id); - index_registry->GetEntry(0); - - CHECK(event_value == 10ULL); - CHECK(seg_value == 11ULL); - REQUIRE(product_id != nullptr); - CHECK(*product_id == "DeclaredProduct|UnitTest|"); - - file->Close(); -} diff --git a/test/form/form_test_with_output.jsonnet b/test/form/form_test_with_output.jsonnet deleted file mode 100644 index ee04ffb58..000000000 --- a/test/form/form_test_with_output.jsonnet +++ /dev/null @@ -1,33 +0,0 @@ -{ - driver: { - cpp: 'generate_layers', - layers: { - event: { total: 10 }, - }, - }, - sources: { - provider: { - cpp: 'ij_source', - }, - }, - modules: { - add: { - cpp: 'module', - }, - form_output: { - cpp: 'form_module', - // Products written by the form module - products: ['sum', 'i', 'j'], - }, - }, - - // Optional outputs mapping: file -> products written into that file. - // Some test runners/CLI invocations can pick this up to produce a concrete - // ROOT output file for easy inspection (e.g. "output.root"). - outputs: [ - { - file: 'output.root', - products: ['sum', 'i', 'j'], - }, - ], -} diff --git a/test/form/writer.cpp b/test/form/writer.cpp index ff71d38b8..8f109102d 100644 --- a/test/form/writer.cpp +++ b/test/form/writer.cpp @@ -7,11 +7,6 @@ #include "test_utils.hpp" #include "toy_tracker.hpp" -#include -#include -#include -#include - #include #include #include @@ -19,8 +14,6 @@ #include #include #include -#include -#include #include static int const NUMBER_EVENT = 4; @@ -68,428 +61,96 @@ int main(int argc, char** argv) tech_config.container_settings[form::technology::ROOT_RNTUPLE]["Toy_Tracker/trackStartPoints"] .emplace_back("force_streamer_field", "true"); - { - form::experimental::form_writer_interface form(config_items, tech_config); - - ToyTracker tracker(4 * 1024); - - // Open checksum file for writing - std::ofstream checksum_file(checksum_filename); - if (!checksum_file.is_open()) { - std::cerr << "ERROR: Could not open checksum file: " << checksum_filename << '\n'; - return 1; - } - - for (int nevent = 0; nevent < NUMBER_EVENT; nevent++) { - std::cout << "PHLEX: Write Event No. " << nevent << '\n'; - - std::vector track_x; - - for (int nseg = 0; nseg < NUMBER_SEGMENT; nseg++) { - - std::vector track_start_x; - generate(track_start_x, 4 * 1024 /* * 1024*/); // sub-event processing - float check = 0.0; - for (float val : track_start_x) - check += val; - - std::string const seg_id_text = std::format("[EVENT={:08X};SEG={:08X}]", nevent, nseg); - - std::string const& segment_id = seg_id_text; - - std::vector products; - std::string const creator = "Toy_Tracker"; - - form::experimental::product_with_name pb = { - "trackStart", &track_start_x, &typeid(std::vector)}; - products.push_back(pb); - - std::vector track_n_hits(std::from_range, std::views::iota(0, 100)); - for (int val : track_n_hits) - check += static_cast(val); - std::cout << "PHLEX: Segment = " << nseg << ": seg_id_text = " << seg_id_text - << ", check = " << check << '\n'; + form::experimental::form_writer_interface form(config_items, tech_config); - form::experimental::product_with_name pb_int = { - "trackNumberHits", &track_n_hits, &typeid(std::vector)}; - products.push_back(pb_int); + ToyTracker tracker(4 * 1024); - std::vector start_points = tracker(); - TrackStart checkPoints; - for (TrackStart const& point : start_points) - checkPoints += point; - std::cout << "PHLEX: Segment = " << nseg << ": seg_id_text = " << seg_id_text - << ", checkPoints = " << checkPoints << '\n'; - - form::experimental::product_with_name pb_points = { - "trackStartPoints", &start_points, &typeid(std::vector)}; - products.push_back(pb_points); + // Open checksum file for writing + std::ofstream checksum_file(checksum_filename); + if (!checksum_file.is_open()) { + std::cerr << "ERROR: Could not open checksum file: " << checksum_filename << '\n'; + return 1; + } - form.write(creator, segment_id, products); + for (int nevent = 0; nevent < NUMBER_EVENT; nevent++) { + std::cout << "PHLEX: Write Event No. " << nevent << '\n'; - // Save segment checksums - checksum_file << std::setprecision(10) << "SEG " << nevent << " " << nseg << " " << check - << " " << checkPoints.getX() << " " << checkPoints.getY() << " " - << checkPoints.getZ() << "\n"; - track_x.insert(track_x.end(), track_start_x.begin(), track_start_x.end()); - } + std::vector track_x; - std::cout << "PHLEX: Write Event segments done " << nevent << '\n'; + for (int nseg = 0; nseg < NUMBER_SEGMENT; nseg++) { + std::vector track_start_x; + generate(track_start_x, 4 * 1024 /* * 1024*/); // sub-event processing float check = 0.0; - for (float val : track_x) + for (float val : track_start_x) check += val; - std::string const evt_id_text = std::format("[EVENT={:08X}]", nevent); + std::string const seg_id_text = std::format("[EVENT={:08X};SEG={:08X}]", nevent, nseg); - std::string const& event_id = evt_id_text; + std::string const& segment_id = seg_id_text; - std::string const creator = "Toy_Tracker_Event"; + std::vector products; + std::string const creator = "Toy_Tracker"; form::experimental::product_with_name pb = { - "trackStartX", &track_x, &typeid(std::vector)}; - std::cout << "PHLEX: Event = " << nevent << ": evt_id_text = " << evt_id_text - << ", check = " << check << '\n'; - - form.write(creator, event_id, pb); - - // Save event checksum - checksum_file << std::setprecision(10) << "EVT " << nevent << " " << check << "\n"; - std::cout << "PHLEX: Write Event done " << nevent << '\n'; - } - - checksum_file.close(); - std::cout << "PHLEX: Write done. Checksums saved to " << checksum_filename << '\n'; - - // Finalize to write FileCatalog metadata - form.finalize(); - std::cout << "PHLEX: Finalize done. FileCatalog written to file." << '\n'; - } - - // Verify that the generated FileUUID exists and is a canonical UUID string. - std::unique_ptr root_file(TFile::Open(filename.c_str(), "READ")); - - if (root_file == nullptr || root_file->IsZombie()) { - std::cerr << "ERROR: Could not open generated ROOT file for validation: " << filename << '\n'; - return 1; - } - - TTree* catalog = root_file->Get("FileCatalog"); - if (catalog == nullptr) { - std::cerr << "ERROR: FileCatalog tree not found in generated ROOT file." << '\n'; - return 1; - } - - std::string* fileUUID = nullptr; - int fileFormatVersion = -1; - catalog->SetBranchAddress("FileUUID", &fileUUID); - catalog->SetBranchAddress("FileFormatVersion", &fileFormatVersion); - if (catalog->GetEntries() < 1) { - std::cerr << "ERROR: FileCatalog tree has no entries." << '\n'; - return 1; - } - catalog->GetEntry(0); - if (fileUUID == nullptr) { - std::cerr << "ERROR: FileUUID branch did not populate a valid pointer." << '\n'; - return 1; - } - std::string fileUUIDValue = *fileUUID; - - TUUID uuidObj(fileUUIDValue.c_str()); - if (uuidObj == TUUID()) { - std::cerr << "ERROR: FileUUID is not valid: " << fileUUIDValue << '\n'; - return 1; - } - - std::cout << "PHLEX: FileUUID validated: " << fileUUIDValue << " (version=" << fileFormatVersion - << ")" << '\n'; - - TTree* registry = root_file->Get("ProductRegistry"); - if (registry == nullptr) { - std::cerr << "ERROR: ProductRegistry tree not found in generated ROOT file." << '\n'; - return 1; - } - - std::string* productName = nullptr; - std::string* processName = nullptr; - std::string* producer = nullptr; - std::string* productID = nullptr; - registry->SetBranchAddress("ProductName", &productName); - registry->SetBranchAddress("ProcessName", &processName); - registry->SetBranchAddress("Producer", &producer); - registry->SetBranchAddress("ProductID", &productID); - - if (registry->GetEntries() == 0) { - std::cerr << "ERROR: ProductRegistry tree has no entries." << '\n'; - return 1; - } - - std::set product_names; - std::set product_ids; - for (int entry = 0; entry < registry->GetEntries(); ++entry) { - registry->GetEntry(entry); - if (productName == nullptr || processName == nullptr || producer == nullptr || - productID == nullptr) { - std::cerr << "ERROR: ProductRegistry branches did not populate valid pointers." << '\n'; - return 1; - } - std::string const expected_product_id = *productName + "|" + *producer + "|" + *processName; - if (*productID != expected_product_id) { - std::cerr << "ERROR: ProductRegistry ProductID mismatch. expected='" << expected_product_id - << "' got='" << *productID << "'." << '\n'; - return 1; - } - product_names.insert(*productName); - product_ids.insert(*productID); - std::cout << "PHLEX: ProductRegistry entry: ProductName='" << *productName << "' ProcessName='" - << *processName << "' Producer='" << *producer << "' ProductID='" << *productID - << "'\n"; - } - - if (product_names.empty()) { - std::cerr << "ERROR: ProductRegistry tree contains no product names." << '\n'; - return 1; - } + "trackStart", &track_start_x, &typeid(std::vector)}; + products.push_back(pb); - std::cout << "PHLEX: ProductRegistry validated: "; - for (auto const& name : product_names) - std::cout << name << " "; - std::cout << '\n'; + std::vector track_n_hits(std::from_range, std::views::iota(0, 100)); + for (int val : track_n_hits) + check += static_cast(val); + std::cout << "PHLEX: Segment = " << nseg << ": seg_id_text = " << seg_id_text + << ", check = " << check << '\n'; - TTree* index_registry = root_file->Get("IndexRegistry"); - if (index_registry == nullptr) { - std::cerr << "ERROR: IndexRegistry tree not found in generated ROOT file." << '\n'; - return 1; - } + form::experimental::product_with_name pb_int = { + "trackNumberHits", &track_n_hits, &typeid(std::vector)}; + products.push_back(pb_int); - TObjString* layer_schema_meta = nullptr; - auto* user_info = index_registry->GetUserInfo(); - if (user_info != nullptr) { - for (int i = 0; i < user_info->GetEntries(); ++i) { - auto* obj = user_info->At(i); - auto* candidate = dynamic_cast(obj); - if (candidate == nullptr) { - continue; - } - std::string payload = candidate->GetString().Data(); - if (payload.size() >= 2 && payload.front() == '[' && payload.back() == ']') { - layer_schema_meta = candidate; - break; - } - } - } + std::vector start_points = tracker(); + TrackStart checkPoints; + for (TrackStart const& point : start_points) + checkPoints += point; + std::cout << "PHLEX: Segment = " << nseg << ": seg_id_text = " << seg_id_text + << ", checkPoints = " << checkPoints << '\n'; - if (layer_schema_meta == nullptr) { - std::cerr << "ERROR: IndexRegistry header does not contain LayerSchema metadata." << '\n'; - return 1; - } + form::experimental::product_with_name pb_points = { + "trackStartPoints", &start_points, &typeid(std::vector)}; + products.push_back(pb_points); - std::vector header_schema; - { - std::string schema_text = layer_schema_meta->GetString().Data(); - if (schema_text.size() >= 2 && schema_text.front() == '[' && schema_text.back() == ']') { - schema_text = schema_text.substr(1, schema_text.size() - 2); - } + form.write(creator, segment_id, products); - std::size_t start = 0; - while (start <= schema_text.size()) { - std::size_t end = schema_text.find(',', start); - std::string token = - schema_text.substr(start, end == std::string::npos ? std::string::npos : end - start); - if (token.size() >= 2 && token.front() == '"' && token.back() == '"') { - token = token.substr(1, token.size() - 2); - } - if (!token.empty()) { - header_schema.push_back(token); - } - if (end == std::string::npos) { - break; - } - start = end + 1; + // Save segment checksums + checksum_file << std::setprecision(10) << "SEG " << nevent << " " << nseg << " " << check + << " " << checkPoints.getX() << " " << checkPoints.getY() << " " + << checkPoints.getZ() << "\n"; + track_x.insert(track_x.end(), track_start_x.begin(), track_start_x.end()); } - } - - if (header_schema.empty()) { - std::cerr << "ERROR: IndexRegistry LayerSchema header is empty." << '\n'; - return 1; - } - - if (index_registry->GetEntries() == 0) { - std::cerr << "ERROR: IndexRegistry tree has no entries." << '\n'; - return 1; - } - auto* branch_list = index_registry->GetListOfBranches(); - if (branch_list == nullptr) { - std::cerr << "ERROR: IndexRegistry has no branch list." << '\n'; - return 1; - } - if (branch_list->GetEntries() != static_cast(header_schema.size()) + 3) { - std::cerr << "ERROR: IndexRegistry branch count does not match LayerSchema size + ProductID + " - "ContainerName + PayloadRow." - << '\n'; - return 1; - } - for (std::size_t i = 0; i < header_schema.size(); ++i) { - auto* branch_obj = branch_list->At(static_cast(i)); - if (branch_obj == nullptr) { - std::cerr << "ERROR: IndexRegistry has null branch object at position " << i << "." << '\n'; - return 1; - } - if (header_schema[i] != branch_obj->GetName()) { - std::cerr << "ERROR: IndexRegistry branch order does not match LayerSchema at position " << i - << ": expected '" << header_schema[i] << "' got '" << branch_obj->GetName() << "'." - << '\n'; - return 1; - } - } - auto* product_branch_obj = branch_list->At(static_cast(header_schema.size())); - if (product_branch_obj == nullptr || std::string(product_branch_obj->GetName()) != "ProductID") { - std::cerr << "ERROR: IndexRegistry branch order missing ProductID after layer branches." - << '\n'; - return 1; - } - auto* container_branch_obj = branch_list->At(static_cast(header_schema.size()) + 1); - if (container_branch_obj == nullptr || - std::string(container_branch_obj->GetName()) != "ContainerName") { - std::cerr << "ERROR: IndexRegistry branch order missing ContainerName after ProductID." << '\n'; - return 1; - } - auto* payload_row_branch_obj = branch_list->At(static_cast(header_schema.size()) + 2); - if (payload_row_branch_obj == nullptr || - std::string(payload_row_branch_obj->GetName()) != "PayloadRow") { - std::cerr << "ERROR: IndexRegistry branch order missing PayloadRow after ContainerName." - << '\n'; - return 1; - } + std::cout << "PHLEX: Write Event segments done " << nevent << '\n'; - std::vector layer_branch_values(header_schema.size(), 0); - for (std::size_t i = 0; i < header_schema.size(); ++i) { - if (index_registry->GetBranch(header_schema[i].c_str()) == nullptr) { - std::cerr << "ERROR: IndexRegistry branch missing for layer '" << header_schema[i] << "'." - << '\n'; - return 1; - } - index_registry->SetBranchAddress(header_schema[i].c_str(), &layer_branch_values[i]); - } - std::string* product_id = nullptr; - if (index_registry->GetBranch("ProductID") == nullptr) { - std::cerr << "ERROR: IndexRegistry ProductID branch missing." << '\n'; - return 1; - } - index_registry->SetBranchAddress("ProductID", &product_id); - std::string* container_name = nullptr; - if (index_registry->GetBranch("ContainerName") == nullptr) { - std::cerr << "ERROR: IndexRegistry ContainerName branch missing." << '\n'; - return 1; - } - index_registry->SetBranchAddress("ContainerName", &container_name); - unsigned long long payload_row = 0; - if (index_registry->GetBranch("PayloadRow") == nullptr) { - std::cerr << "ERROR: IndexRegistry PayloadRow branch missing." << '\n'; - return 1; - } - index_registry->SetBranchAddress("PayloadRow", &payload_row); - - Long64_t const sample_entries = - index_registry->GetEntries() < 8 ? index_registry->GetEntries() : 8; - unsigned long long prev_event = 0; - unsigned long long prev_seg = 0; - std::set observed_product_ids; - bool has_prev = false; - for (int entry = 0; entry < index_registry->GetEntries(); ++entry) { - index_registry->GetEntry(entry); - if (layer_branch_values.empty()) { - std::cerr << "ERROR: IndexRegistry layer branch values are empty." << '\n'; - return 1; - } - if (product_id == nullptr || product_id->empty()) { - std::cerr << "ERROR: IndexRegistry ProductID branch did not populate valid value." << '\n'; - return 1; - } - if (container_name == nullptr || container_name->empty()) { - std::cerr << "ERROR: IndexRegistry ContainerName branch did not populate valid value." - << '\n'; - return 1; - } - // ContainerName must be a top-level TTree/RNTuple name (no '/' slash) - if (container_name->find('/') != std::string::npos) { - std::cerr << "ERROR: IndexRegistry ContainerName should be top-level container name, got: '" - << *container_name << "'." << '\n'; - return 1; - } + float check = 0.0; + for (float val : track_x) + check += val; - unsigned long long const expected_payload_row = static_cast(entry); - if (payload_row != expected_payload_row) { - std::cerr << "ERROR: IndexRegistry PayloadRow mismatch at entry " << entry << ": expected " - << expected_payload_row << " got " << payload_row << "." << '\n'; - return 1; - } + std::string const evt_id_text = std::format("[EVENT={:08X}]", nevent); - observed_product_ids.insert(*product_id); - - if (entry < sample_entries && header_schema.size() >= 2) { - unsigned long long const event_val = layer_branch_values[0]; - unsigned long long const seg_val = layer_branch_values[1]; - - if (event_val >= static_cast(NUMBER_EVENT)) { - std::cerr << "ERROR: EVENT value out of range in IndexRegistry sample: " << event_val - << '\n'; - return 1; - } - if (seg_val >= static_cast(NUMBER_SEGMENT)) { - std::cerr << "ERROR: SEG value out of range in IndexRegistry sample: " << seg_val << '\n'; - return 1; - } - - if (has_prev) { - bool const monotonic_ok = (event_val > prev_event && seg_val == 0) || - (event_val == prev_event && seg_val > prev_seg) || - (event_val == prev_event && seg_val == prev_seg); - if (!monotonic_ok) { - std::cerr - << "ERROR: IndexRegistry sample entries are not monotonic by EVENT/SEG ordering." - << '\n'; - return 1; - } - } - - prev_event = event_val; - prev_seg = seg_val; - has_prev = true; - } - } + std::string const& event_id = evt_id_text; - if (header_schema.size() != 2 || header_schema[0] != "EVENT" || header_schema[1] != "SEG") { - std::cerr - << "ERROR: IndexRegistry LayerSchema header is not EVENT,SEG as expected for this test." - << '\n'; - return 1; - } + std::string const creator = "Toy_Tracker_Event"; - if (product_ids.empty()) { - std::cerr << "ERROR: ProductRegistry ProductID values are empty." << '\n'; - return 1; - } + form::experimental::product_with_name pb = { + "trackStartX", &track_x, &typeid(std::vector)}; + std::cout << "PHLEX: Event = " << nevent << ": evt_id_text = " << evt_id_text + << ", check = " << check << '\n'; - if (observed_product_ids.empty()) { - std::cerr << "ERROR: IndexRegistry ProductID values are empty." << '\n'; - return 1; - } + form.write(creator, event_id, pb); - for (auto const& observed_id : observed_product_ids) { - if (!product_ids.contains(observed_id)) { - std::cerr << "ERROR: IndexRegistry ProductID value not present in ProductRegistry: " - << observed_id << '\n'; - return 1; - } + // Save event checksum + checksum_file << std::setprecision(10) << "EVT " << nevent << " " << check << "\n"; + std::cout << "PHLEX: Write Event done " << nevent << '\n'; } - std::cout << "PHLEX: ProductRegistry validated with " << product_names.size() - << " product names and " << product_ids.size() << " product IDs" << '\n'; - - std::cout << "PHLEX: IndexRegistry validated with " << index_registry->GetEntries() << " entries" - << '\n'; - + checksum_file.close(); + std::cout << "PHLEX: Write done. Checksums saved to " << checksum_filename << '\n'; return 0; } From 8c9f24f30485f183c8915779f7dc317821c05438 Mon Sep 17 00:00:00 2001 From: Wanwei Wu Date: Thu, 25 Jun 2026 13:57:48 -0500 Subject: [PATCH 2/2] remove remaining getEntryCount introduced for metadata --- form/root_storage/root_rfield_write_container.cpp | 6 ------ form/root_storage/root_rfield_write_container.hpp | 1 - 2 files changed, 7 deletions(-) diff --git a/form/root_storage/root_rfield_write_container.cpp b/form/root_storage/root_rfield_write_container.cpp index 1c87e8346..a52ba5ad7 100644 --- a/form/root_storage/root_rfield_write_container.cpp +++ b/form/root_storage/root_rfield_write_container.cpp @@ -130,10 +130,4 @@ namespace form::detail::experimental { m_rntuple_parent->m_model->AddField(std::move(field)); } - std::uint64_t ROOT_RField_Write_ContainerImp::getEntryCount() - { - if (m_rntuple_parent && m_rntuple_parent->m_writer) - return m_rntuple_parent->m_writer->GetNEntries(); - return 0; - } } diff --git a/form/root_storage/root_rfield_write_container.hpp b/form/root_storage/root_rfield_write_container.hpp index c73f4c1f4..d5387582b 100644 --- a/form/root_storage/root_rfield_write_container.hpp +++ b/form/root_storage/root_rfield_write_container.hpp @@ -25,7 +25,6 @@ namespace form::detail::experimental { void setParent(std::shared_ptr const parent) override; void fill(void const* data) override; void commit() override; - std::uint64_t getEntryCount() override; private: std::shared_ptr m_tfile;