Skip to content

FORM RNTuple Draft - #635

Merged
wwuoneway merged 75 commits into
Framework-R-D:mainfrom
aolivier23:form_rntuple_for_inactive_pr
Jun 18, 2026
Merged

FORM RNTuple Draft#635
wwuoneway merged 75 commits into
Framework-R-D:mainfrom
aolivier23:form_rntuple_for_inactive_pr

Conversation

@aolivier23

@aolivier23 aolivier23 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Adding an early draft of RNTuple for FORM. This PR adds the capability to write data products to a file using RNTuple instead of TTree, but it is disabled by default through a CMake flag. The goal is for the RNTuple backend to become part of our maintenance routine while we prepare it for v0.4.

  • Revert github actions references to Framework-R-D/phlex@main before merging
  • CI / Workflows

    • Added form-rntuple-storage input to .github/actions/configure-cmake/action.yaml, exposed as env FORM_RNTUPLE_STORAGE, and pass -DFORM_USE_RNTUPLE_STORAGE="$FORM_RNTUPLE_STORAGE" to cmake.
    • Workflows updated in .github/workflows/cmake-build.yaml and .github/workflows/coverage.yaml to reference the branch-local configure-cmake action (aolivier23/phlex/.github/actions/configure-cmake@form_rntuple_for_inactive_pr) for the PR.
    • CodeQL warnings: seven alerts about unpinned third-party GitHub Action refs (files in .github/actions/* reference @main). Author notes a checklist item to revert action refs to Framework-R-D/phlex@main before merge.
  • Build system

    • New CMake option FORM_USE_RNTUPLE_STORAGE added to form/CMakeLists.txt (default OFF).
    • form/root_storage/CMakeLists.txt refactored to:
      • conditionally append RNTuple-related sources when FORM_USE_RNTUPLE_STORAGE is enabled,
      • set USE_RNTUPLE_STORAGE compile definition,
      • include ROOT::ROOTNTuple in link deps when enabled.
    • Tests and test gating updated so RNTuple tests are only added when FORM_USE_ROOT_STORAGE and FORM_USE_RNTUPLE_STORAGE are enabled.
  • Code — RNTuple storage implementation (experimental / draft)

    • Read container
      • Added header and implementation for form::detail::experimental::ROOT_RField_Read_ContainerImp.
      • Lazily opens ROOT::RNTupleReader, constructs ROOT::RNTupleView<void>, bounds-checks ids, allocates per-entry buffers, performs type-fallback for streamer/type mismatches, converts ROOT exceptions to std::runtime_error, and returns per-entry data via caller pointer.
    • Write containers
      • Added header and implementation for ROOT_RField_Write_ContainerImp: per-field RField write container; supports force_streamer_field, lazily creates writer/entry on first fill, binds raw pointers, and commits via parent writer.
      • Added header and implementation for ROOT_RNTuple_Write_ContainerImp: parent/association for RNTuple writing. Constructor/destructor present; core methods (setupWrite, fill, commit) are currently stubbed or throw — write-side functionality incomplete.
    • Factories
      • form/util/factories.hpp extended to return RNTuple read/write implementations (ROOT_RNTuple_Write_ContainerImp, ROOT_RField_Read_ContainerImp, ROOT_RField_Write_ContainerImp) when the detected ROOT minor indicates RNTuple support.
  • Tests

    • test/form/CMakeLists.txt updated to:
      • include ROOT_RNTUPLE in FORM_TEST_TECHNOLOGIES when enabled,
      • parameterize artifact/checksum/log/test-workdir names with _${TECH},
      • add TEST_PROPERTIES resource lock for form_storage_test_${TECH},
      • add a gated form_module_rntuple cet_test conditioned on FORM_USE_ROOT_STORAGE AND FORM_USE_RNTUPLE_STORAGE.
    • New test config test/form/form_test_rntuple.jsonnet added to exercise ROOT_RNTUPLE write path.
    • form_root_schema_write_test.cpp and form_root_schema_read_test.cpp modified to accept a technology string argument and write technology-specific log files.
    • test/form/form_storage_test.cpp expanded with additional lifecycle and error-handling tests, including a new TEST_CASE asserting unsupported attributes throw std::runtime_error.
  • Code quality / maintenance notes

    • RNTuple backend is experimental and gated by CMake flag; several write-side methods remain unimplemented and require follow-up to complete write functionality.
    • One commit removed a new assertion requiring id >= 0 because tests use id < 0 (test-compatibility fix).
    • Action refs in composite actions/workflows are currently branch-local for testing; they must be reverted/pinned to the canonical repository/tag before merging to address CodeQL warnings and ensure stable CI.

runtime dispatch problems that were stopping RNTuple code from being
used.
choosing a technology from the command line to match form_module.  The
form_basics test still needs an update.
…ming testing commit. Bugs had to do with types for which no TClass exists and calling functions in the wrong order.
… technology because phlex's clang-tidy configuration rightly complains about initialization of a global variable throwing an exception.
…ild directory can ignore this if I'm not careful to reset it.
…to match the rest of phlex. Added a comment clarifying why this can't be done for RField read container. Removed non-functional ROOT_RField_read_container::setAttribute().
@aolivier23
aolivier23 requested review from gemmeren and wwuoneway June 4, 2026 19:12
@aolivier23

Copy link
Copy Markdown
Contributor Author

@phlexbot format

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Format Fixes Applied

✅ clang-format fixes pushed (commit a89502d)
✅ cmake-format fixes pushed (commit a263e7f)

⚠️ Note: Some issues may require manual review.

@greenc-FNAL

greenc-FNAL commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

31 fixed, 0 new since branch point (db22b8a)
31 fixed, 0 new since previous report on PR (063c2ee)

✅ 31 CodeQL alerts resolved since the previous PR commit

  • Warning # 196 actions/untrusted-checkout-toctou/critical at .github/workflows/clang-tidy-fix.yaml:109:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 197 actions/untrusted-checkout-toctou/critical at .github/workflows/clang-tidy-fix.yaml:137:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 198 actions/untrusted-checkout-toctou/critical at .github/workflows/clang-tidy-fix.yaml:157:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 199 actions/untrusted-checkout-toctou/critical at .github/workflows/clang-tidy-fix.yaml:201:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 200 actions/untrusted-checkout-toctou/critical at .github/workflows/coverage.yaml:166:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 201 actions/untrusted-checkout-toctou/critical at .github/workflows/coverage.yaml:188:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 202 actions/untrusted-checkout-toctou/critical at .github/workflows/coverage.yaml:207:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 203 actions/untrusted-checkout-toctou/critical at .github/workflows/coverage.yaml:226:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 204 actions/untrusted-checkout-toctou/critical at .github/workflows/coverage.yaml:251:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 205 actions/untrusted-checkout-toctou/critical at .github/workflows/header-guards-fix.yaml:107:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 206 actions/untrusted-checkout-toctou/critical at .github/workflows/markdown-fix.yaml:107:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 207 actions/untrusted-checkout-toctou/critical at .github/workflows/yaml-fix.yaml:108:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 208 actions/untrusted-checkout-toctou/critical at .github/workflows/yaml-fix.yaml:111:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 209 actions/untrusted-checkout-toctou/high at .github/workflows/clang-format-fix.yaml:94:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 210 actions/untrusted-checkout-toctou/high at .github/workflows/cmake-format-fix.yaml:94:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 211 actions/untrusted-checkout-toctou/high at .github/workflows/jsonnet-format-fix.yaml:95:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 212 actions/untrusted-checkout-toctou/high at .github/workflows/python-fix.yaml:94:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 213 actions/untrusted-checkout/high at .github/workflows/clang-format-fix.yaml:94:9 — Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
    Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
  • Warning # 214 actions/untrusted-checkout/high at .github/workflows/cmake-format-fix.yaml:94:9 — Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
    Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
  • Warning # 215 actions/untrusted-checkout/high at .github/workflows/jsonnet-format-fix.yaml:95:9 — Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
    Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
  • ✅ …and 11 more alerts (see Code Scanning for the full list).

✅ 31 CodeQL alerts resolved since the branch point

  • Warning # 196 actions/untrusted-checkout-toctou/critical at .github/workflows/clang-tidy-fix.yaml:109:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 197 actions/untrusted-checkout-toctou/critical at .github/workflows/clang-tidy-fix.yaml:137:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 198 actions/untrusted-checkout-toctou/critical at .github/workflows/clang-tidy-fix.yaml:157:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 199 actions/untrusted-checkout-toctou/critical at .github/workflows/clang-tidy-fix.yaml:201:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 200 actions/untrusted-checkout-toctou/critical at .github/workflows/coverage.yaml:166:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 201 actions/untrusted-checkout-toctou/critical at .github/workflows/coverage.yaml:188:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 202 actions/untrusted-checkout-toctou/critical at .github/workflows/coverage.yaml:207:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 203 actions/untrusted-checkout-toctou/critical at .github/workflows/coverage.yaml:226:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 204 actions/untrusted-checkout-toctou/critical at .github/workflows/coverage.yaml:251:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 205 actions/untrusted-checkout-toctou/critical at .github/workflows/header-guards-fix.yaml:107:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 206 actions/untrusted-checkout-toctou/critical at .github/workflows/markdown-fix.yaml:107:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 207 actions/untrusted-checkout-toctou/critical at .github/workflows/yaml-fix.yaml:108:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 208 actions/untrusted-checkout-toctou/critical at .github/workflows/yaml-fix.yaml:111:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 209 actions/untrusted-checkout-toctou/high at .github/workflows/clang-format-fix.yaml:94:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 210 actions/untrusted-checkout-toctou/high at .github/workflows/cmake-format-fix.yaml:94:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 211 actions/untrusted-checkout-toctou/high at .github/workflows/jsonnet-format-fix.yaml:95:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 212 actions/untrusted-checkout-toctou/high at .github/workflows/python-fix.yaml:94:9 — Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
    Insufficient protection against execution of untrusted code on a privileged workflow (issue_comment).
  • Warning # 213 actions/untrusted-checkout/high at .github/workflows/clang-format-fix.yaml:94:9 — Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
    Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
  • Warning # 214 actions/untrusted-checkout/high at .github/workflows/cmake-format-fix.yaml:94:9 — Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
    Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
  • Warning # 215 actions/untrusted-checkout/high at .github/workflows/jsonnet-format-fix.yaml:95:9 — Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
    Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: issue_comment).
  • ✅ …and 11 more alerts (see Code Scanning for the full list).

Review the full CodeQL report for details.

@github-actions

This comment was marked as outdated.

@codecov

codecov Bot commented Jun 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.75182% with 25 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
form/root_storage/root_rfield_write_container.cpp 77.94% 11 Missing and 4 partials ⚠️
form/root_storage/root_rfield_read_container.cpp 84.78% 4 Missing and 3 partials ⚠️
form/util/factories.hpp 75.00% 0 Missing and 3 partials ⚠️
@@            Coverage Diff             @@
##             main     #635      +/-   ##
==========================================
+ Coverage   83.27%   83.96%   +0.69%     
==========================================
  Files         162      170       +8     
  Lines        5912     6605     +693     
  Branches      670      798     +128     
==========================================
+ Hits         4923     5546     +623     
- Misses        796      819      +23     
- Partials      193      240      +47     
Flag Coverage Δ
scripts 78.86% <ø> (+2.49%) ⬆️
unittests 86.36% <81.75%> (-0.49%) ⬇️

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

Files with missing lines Coverage Δ
form/root_storage/root_rfield_write_container.hpp 100.00% <100.00%> (ø)
form/root_storage/root_rntuple_write_container.cpp 100.00% <100.00%> (ø)
form/storage/storage_writer.cpp 81.68% <100.00%> (-3.43%) ⬇️
form/util/factories.hpp 61.53% <75.00%> (+17.09%) ⬆️
form/root_storage/root_rfield_read_container.cpp 84.78% <84.78%> (ø)
form/root_storage/root_rfield_write_container.cpp 77.94% <77.94%> (ø)

... and 7 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

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

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

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@aolivier23

Copy link
Copy Markdown
Contributor Author

Awaiting Framework-R-D/action-configure-cmake#5 for final test run

…nullptr. Also took out the container commit in form_storage_test when this happens because a commit without a fill is an error.
@aolivier23

Copy link
Copy Markdown
Contributor Author

@phlexbot format

@github-actions

Copy link
Copy Markdown
Contributor

Format Fixes Applied

✅ clang-format fixes pushed (commit 6c9615c)

⚠️ Note: Some issues may require manual review.

@github-actions

This comment was marked as outdated.

@github-actions

Copy link
Copy Markdown
Contributor

Clang-Tidy Check Results

Found 6048 issue(s); none are newly introduced by this patch.

All issues by check:

  • readability-identifier-naming: 1861
  • readability-redundant-typename: 1223
  • readability-redundant-member-init: 1208
  • performance-unnecessary-value-param: 476
  • readability-avoid-const-params-in-decls: 257
  • modernize-pass-by-value: 200
  • readability-use-concise-preprocessor-directives: 189
  • modernize-use-designated-initializers: 102
  • readability-braces-around-statements: 91
  • readability-convert-member-functions-to-static: 80
  • readability-const-return-type: 47
  • modernize-use-auto: 36
  • readability-qualified-auto: 25
  • modernize-avoid-c-style-cast: 22
  • readability-inconsistent-ifelse-braces: 21
  • readability-redundant-control-flow: 20
  • readability-math-missing-parentheses: 17
  • modernize-avoid-c-arrays: 15
  • modernize-use-equals-default: 14
  • modernize-use-using: 13
  • readability-static-definition-in-anonymous-namespace: 12
  • readability-function-size: 12
  • bugprone-branch-clone: 12
  • modernize-return-braced-init-list: 11
  • modernize-concat-nested-namespaces: 10
  • readability-isolate-declaration: 8
  • cppcoreguidelines-special-member-functions: 8
  • readability-container-contains: 8
  • modernize-use-starts-ends-with: 7
  • readability-container-size-empty: 7
  • modernize-use-ranges: 6
  • readability-inconsistent-declaration-parameter-name: 5
  • readability-redundant-access-specifiers: 5
  • modernize-use-std-numbers: 3
  • readability-redundant-casting: 3
  • modernize-use-emplace: 3
  • clang-analyzer-security.ArrayBound: 2
  • readability-simplify-boolean-expr: 1
  • modernize-use-integer-sign-comparison: 1
  • readability-use-anyofallof: 1
  • modernize-make-shared: 1
  • performance-avoid-endl: 1
  • readability-redundant-string-init: 1
  • modernize-redundant-void-arg: 1
  • performance-move-const-arg: 1
  • readability-else-after-return: 1

See inline comments for details. Comment @phlexbot tidy-fix to auto-fix.

@github-actions

Copy link
Copy Markdown
Contributor

Clang-Tidy Check Results

Found 6358 issue(s); none are newly introduced by this patch.

All issues by check:

  • readability-identifier-naming: 2132
  • readability-redundant-typename: 1223
  • readability-redundant-member-init: 1208
  • performance-unnecessary-value-param: 476
  • readability-avoid-const-params-in-decls: 269
  • modernize-pass-by-value: 206
  • readability-use-concise-preprocessor-directives: 189
  • modernize-use-designated-initializers: 102
  • readability-braces-around-statements: 93
  • readability-convert-member-functions-to-static: 80
  • readability-const-return-type: 50
  • modernize-use-auto: 36
  • readability-qualified-auto: 26
  • readability-redundant-control-flow: 24
  • modernize-avoid-c-style-cast: 23
  • readability-inconsistent-ifelse-braces: 21
  • modernize-concat-nested-namespaces: 17
  • readability-math-missing-parentheses: 17
  • modernize-use-equals-default: 15
  • modernize-avoid-c-arrays: 15
  • modernize-use-using: 13
  • bugprone-branch-clone: 12
  • readability-function-size: 12
  • readability-static-definition-in-anonymous-namespace: 12
  • modernize-return-braced-init-list: 11
  • readability-container-contains: 8
  • cppcoreguidelines-special-member-functions: 8
  • readability-isolate-declaration: 8
  • readability-container-size-empty: 7
  • modernize-use-starts-ends-with: 7
  • modernize-use-ranges: 6
  • readability-inconsistent-declaration-parameter-name: 5
  • readability-redundant-access-specifiers: 5
  • modernize-use-std-numbers: 3
  • readability-redundant-casting: 3
  • modernize-use-emplace: 3
  • modernize-use-integer-sign-comparison: 2
  • clang-analyzer-security.ArrayBound: 2
  • performance-avoid-endl: 2
  • readability-redundant-string-init: 1
  • performance-move-const-arg: 1
  • modernize-redundant-void-arg: 1
  • readability-else-after-return: 1
  • readability-simplify-boolean-expr: 1
  • readability-use-anyofallof: 1
  • modernize-make-shared: 1

See inline comments for details. Comment @phlexbot tidy-fix to auto-fix.

@wwuoneway

Copy link
Copy Markdown
Contributor

looks good to me

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.

4 participants