Skip to content

Add FORM output module for phlex product_store integration#177

Merged
knoepfel merged 36 commits intomainfrom
barnali/form-integration-work
Dec 18, 2025
Merged

Add FORM output module for phlex product_store integration#177
knoepfel merged 36 commits intomainfrom
barnali/form-integration-work

Conversation

@barnaliy
Copy link
Contributor

@barnaliy barnaliy commented Dec 9, 2025

No description provided.

knoepfel and others added 7 commits December 1, 2025 17:00
Implements FormOutputModule that integrates phlex's product_store with
FORM's persistence layer. The module extracts products from phlex stores
and writes them using configurable FORM backend technologies.

Key changes:
- form_module.cpp: New output module with phlex product_store integration
- Updates to form/CMakeLists.txt to build form_module library
- Links against phlex::module and phlex::experimental interfaces

Note: Requires phlex standalone build modifications (submitted separately)
…ntal directly)

, 2. Create test_helpers.hpp with createTypeMap() implementation, 3. Register TrackStart and vector<TrackStart> types for FORM serialization, 4. Update writer.cpp and reader.cpp to use form::experimental namespace, 5. Tests now build against form library directly without mock layer
@barnaliy
Copy link
Contributor Author

@phlexbot format

@github-actions
Copy link
Contributor

Automatic fixes pushed (commit eeffe13).
⚠️ Note: Some issues may require manual review and fixing.

@github-actions
Copy link
Contributor

No automatic fixes were necessary.

@codecov
Copy link

codecov bot commented Dec 11, 2025

Codecov Report

❌ Patch coverage is 85.52632% with 11 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
form/form_module.cpp 87.50% 6 Missing and 1 partial ⚠️
form/form/form.cpp 88.23% 1 Missing and 1 partial ⚠️
form/root_storage/root_ttree_container.cpp 33.33% 1 Missing and 1 partial ⚠️
@@            Coverage Diff             @@
##             main     #177      +/-   ##
==========================================
+ Coverage   82.01%   82.27%   +0.25%     
==========================================
  Files         120      118       -2     
  Lines        2174     2211      +37     
  Branches      348      353       +5     
==========================================
+ Hits         1783     1819      +36     
- Misses        253      259       +6     
+ Partials      138      133       -5     
Flag Coverage Δ
unittests 82.27% <85.52%> (+0.25%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
form/form/form.hpp 100.00% <ø> (ø)
form/form/form.cpp 77.14% <88.23%> (-3.81%) ⬇️
form/root_storage/root_ttree_container.cpp 46.15% <33.33%> (-3.85%) ⬇️
form/form_module.cpp 87.50% <87.50%> (ø)

... and 6 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57dcdf2...cd323e2. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@barnaliy barnaliy marked this pull request as ready for review December 11, 2025 22:05
@gemmeren gemmeren self-requested a review December 12, 2025 20:20
gemmeren
gemmeren previously approved these changes Dec 12, 2025
…OutputModule by running a Phlex workflow that uses the form_module plugin, improving code coverage for form_module.cpp. The test follows the same pattern as test/plugins, using a jsonnet configuration file to instantiate and run the module through Phlex.
Copy link
Member

@knoepfel knoepfel left a comment

Choose a reason for hiding this comment

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

@barnaliy, please try this change

@pcanal
Copy link
Contributor

pcanal commented Dec 17, 2025

The problem may be that the tree is written in the destructor, but by that time the TFile might already be closed or the directory association lost!

The problem was already solved by c6a1464

@pcanal pcanal self-requested a review December 17, 2025 22:10
This is a tempoary work-around until std::type_info is part of the Phlex API.
Adding support for:
  long, float, double
and
  std::vector of int, long, float and double.
@pcanal
Copy link
Contributor

pcanal commented Dec 17, 2025

@barnaliy @gemmeren I added another commit 9b49214 to extent the list of support types.

If we can address #177 (comment) then we have enough in place to support arbitrary simple configured products (instead of hard coding a single product).

@pcanal pcanal self-requested a review December 17, 2025 22:21
@gemmeren
Copy link
Contributor

Thanks Philippe,
for adding the basic/native types. @barnaliy Maybe bool, char and string can be added as well (can be separate PR).

gemmeren
gemmeren previously approved these changes Dec 17, 2025
…comment)Removed hardcoded 'sum' registration. FormOutputModule now reads product names from the jsonnet config 'products' field, making it generic for any configured products.
@knoepfel
Copy link
Member

@phlexbot format

@github-actions
Copy link
Contributor

No automatic fixes were necessary.

@github-actions
Copy link
Contributor

Automatic fixes pushed (commit d6ae99a).
⚠️ Note: Some issues may require manual review and fixing.

@knoepfel
Copy link
Member

@barnaliy and @pcanal, we now have successful builds and tests. Does anything else need to be done before approving and merging this PR?

@pcanal pcanal self-requested a review December 18, 2025 18:46
Copy link
Contributor

@pcanal pcanal left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@knoepfel knoepfel merged commit ec29fea into main Dec 18, 2025
36 checks passed
@knoepfel knoepfel deleted the barnali/form-integration-work branch December 18, 2025 18:51
@barnaliy
Copy link
Contributor Author

🎉 Thank you all!

@gemmeren
Copy link
Contributor

Thanks for all your work 😃 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integrate FORM writing with Phlex

4 participants