Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions form/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,9 @@ target_link_libraries(form_module PRIVATE phlex::module form)
target_include_directories(form_module PRIVATE ${PROJECT_SOURCE_DIR})

install(TARGETS form_module LIBRARY DESTINATION lib)

add_library(form_source MODULE form_source.cpp)
target_link_libraries(form_source PRIVATE phlex::module form)
target_include_directories(form_source PRIVATE ${PROJECT_SOURCE_DIR})
install(TARGETS form_source LIBRARY DESTINATION lib)

83 changes: 83 additions & 0 deletions form/form_source.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
#include "phlex/source.hpp"

#include "form/config.hpp"
#include "form/form.hpp"
#include "form/technology.hpp"

#include <iostream>
#include <memory>
#include <stdexcept>
#include <string>
#include <string_view>
#include <unordered_map>
#include <vector>

using namespace phlex;
using namespace phlex::experimental::literals;

PHLEX_REGISTER_PROVIDERS(s, config)
{
std::cout << "Registering FORM source providers...\n";

Check warning on line 20 in form/form_source.cpp

View check run for this annotation

Codecov / codecov/patch

form/form_source.cpp#L20

Added line #L20 was not covered by tests

// --- Extract configuration ---
std::string const input_file = config.get<std::string>("input_file");
std::string const creator = config.get<std::string>("creator");
std::string const tech_string = config.get<std::string>("technology", "ROOT_TTREE");

Check warning on line 25 in form/form_source.cpp

View check run for this annotation

Codecov / codecov/patch

form/form_source.cpp#L23-L25

Added lines #L23 - L25 were not covered by tests

std::cout << "Configuration:\n";
std::cout << " input_file: " << input_file << "\n";
std::cout << " creator: " << creator << "\n";
std::cout << " technology: " << tech_string << "\n";

Check warning on line 30 in form/form_source.cpp

View check run for this annotation

Codecov / codecov/patch

form/form_source.cpp#L27-L30

Added lines #L27 - L30 were not covered by tests

// --- Resolve technology enum ---
std::unordered_map<std::string_view, int> const tech_lookup = {
{"ROOT_TTREE", form::technology::ROOT_TTREE},
{"ROOT_RNTUPLE", form::technology::ROOT_RNTUPLE},
{"HDF5", form::technology::HDF5}};

Check warning on line 36 in form/form_source.cpp

View check run for this annotation

Codecov / codecov/patch

form/form_source.cpp#L33-L36

Added lines #L33 - L36 were not covered by tests

auto it = tech_lookup.find(tech_string);

Check warning on line 38 in form/form_source.cpp

View check run for this annotation

Codecov / codecov/patch

form/form_source.cpp#L38

Added line #L38 was not covered by tests
if (it == tech_lookup.end()) {
throw std::runtime_error("Unknown technology: " + tech_string);

Check warning on line 40 in form/form_source.cpp

View check run for this annotation

Codecov / codecov/patch

form/form_source.cpp#L40

Added line #L40 was not covered by tests
}
int const technology = it->second;

Check warning on line 42 in form/form_source.cpp

View check run for this annotation

Codecov / codecov/patch

form/form_source.cpp#L42

Added line #L42 was not covered by tests

// --- Build input config ---
form::experimental::config::output_item_config input_cfg;
form::experimental::config::tech_setting_config tech_cfg;
input_cfg.addItem("i", input_file, technology);
input_cfg.addItem("j", input_file, technology);

Check warning on line 48 in form/form_source.cpp

View check run for this annotation

Codecov / codecov/patch

form/form_source.cpp#L45-L48

Added lines #L45 - L48 were not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of debating what to hardcode ( here, placeholder i and j, or meaningful names), it would be preferable to make the code automatically detect available items from the input file. This would eliminate hardcoding entirely and make the module truly reusable and scalable. A real input file may contain many possible items, and production code should ideally adapt dynamically instead assuming a fixed set. I would suggest moving such hardcoded i and j to test code, while the production code should be intelligent enough to discover available items automatically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current approach is simple and explicit for Prototype 0.1. Auto-discovery is the right long-term goal, but there are two concrete blockers that go beyond the scope of this PR:

  1. product_query fields require compile-time _id literals and do not accept runtime strings, making it impossible to loop over a dynamically discovered list of products
  2. FORM has no enumeration API to discover available items from an input file

I propose keeping the current approach for Prototype 0.1 and tracking auto-discovery as a follow-up issue once these blockers are resolved.


// --- Create shared form_interface ---
auto reader = std::make_shared<form::experimental::form_interface>(input_cfg, tech_cfg);

Check warning on line 51 in form/form_source.cpp

View check run for this annotation

Codecov / codecov/patch

form/form_source.cpp#L51

Added line #L51 was not covered by tests

// --- Register providers ---
// FIXME: Prototype 0.1 -- product names and types hardcoded.
// product_query fields must be compile-time _id literals, not runtime strings.
// To add new products, add new s.provide() blocks below following the same pattern.

s.provide("provide_i",
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this be needed for every data product we read from input? If so, we may want to think about a way to shorten or auto-generate these functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is already noted in the FIXME comment — it's a known limitation of Prototype 0.1 that will be addressed as the type list grows.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here too I wouldn't mind a git issue

[reader, creator](data_cell_index const& id) -> int {
std::cout << "\n=== form_source: providing i ===\n";
form::experimental::product_with_name pb{"i", nullptr, &typeid(int)};
reader->read(creator, id.to_string(), pb);

Check warning on line 62 in form/form_source.cpp

View check run for this annotation

Codecov / codecov/patch

form/form_source.cpp#L58-L62

Added lines #L58 - L62 were not covered by tests
if (!pb.data) {
throw std::runtime_error("FORM read returned null data for product: i");

Check warning on line 64 in form/form_source.cpp

View check run for this annotation

Codecov / codecov/patch

form/form_source.cpp#L64

Added line #L64 was not covered by tests
}
return *static_cast<int const*>(pb.data);
})
.output_product(product_query{.creator = "input"_id, .layer = "event"_id, .suffix = "i"_id});

Check warning on line 68 in form/form_source.cpp

View check run for this annotation

Codecov / codecov/patch

form/form_source.cpp#L66-L68

Added lines #L66 - L68 were not covered by tests

s.provide("provide_j",
[reader, creator](data_cell_index const& id) -> int {
std::cout << "\n=== form_source: providing j ===\n";
form::experimental::product_with_name pb{"j", nullptr, &typeid(int)};
reader->read(creator, id.to_string(), pb);

Check warning on line 74 in form/form_source.cpp

View check run for this annotation

Codecov / codecov/patch

form/form_source.cpp#L70-L74

Added lines #L70 - L74 were not covered by tests
if (!pb.data) {
throw std::runtime_error("FORM read returned null data for product: j");

Check warning on line 76 in form/form_source.cpp

View check run for this annotation

Codecov / codecov/patch

form/form_source.cpp#L76

Added line #L76 was not covered by tests
}
return *static_cast<int const*>(pb.data);
})
.output_product(product_query{.creator = "input"_id, .layer = "event"_id, .suffix = "j"_id});

Check warning on line 80 in form/form_source.cpp

View check run for this annotation

Codecov / codecov/patch

form/form_source.cpp#L78-L80

Added lines #L78 - L80 were not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

The two provider lambdas for i and j are almost identical, with only the product name string differing. Consider extracting the common logic into a factory function or template to reduce code duplication and improve maintainability. This would also make it easier to add more products in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The duplication is not an oversight.
The lambda body duplication is minimal and intentional for clarity in Prototype 0.1
The output_product() call cannot be generalized due to product_query compile-time _id literal constraint — already noted in FIXME.


std::cout << "FORM source providers registered successfully\n";

Check warning on line 82 in form/form_source.cpp

View check run for this annotation

Codecov / codecov/patch

form/form_source.cpp#L82

Added line #L82 was not covered by tests
}
Loading