Skip to content

Adjust driver API#443

Merged
knoepfel merged 9 commits intoFramework-R-D:mainfrom
knoepfel:adjust-api
Mar 27, 2026
Merged

Adjust driver API#443
knoepfel merged 9 commits intoFramework-R-D:mainfrom
knoepfel:adjust-api

Conversation

@knoepfel
Copy link
Copy Markdown
Member

@knoepfel knoepfel commented Mar 23, 2026

This PR includes the changes discussed in #438. Specifically, it allows a registration like:

void cells_to_process(data_cell_cursor const& job) 
{
  unsigned int const num_runs = 2;
  unsigned int const num_subruns = 2;
  unsigned int const num_spills = 3;
  unsigned int const num_calibration_periods = 4;

  for (unsigned int r : std::views::iota(0u, num_runs)) {
    auto run = job.yield_child("run", r);

    // Go over subruns for each run
    for (unsigned int sr : std::views::iota(0u, num_subruns)) {
      auto subrun = run.yield_child("subrun", sr);
      for (unsigned int spill : std::views::iota(0u, num_spills)) {
        subrun.yield_child("spill", spill);
      }
    }

    // Go over calibration periods for each run
    for (unsigned int cp : std::views::iota(0u, num_calibration_periods)) {
      run.yield_child("calibration_period", cp);
    }
  }
}

PHLEX_REGISTER_DRIVER(d, config)
{
  //  We want to express a hierarchy that looks like:
  // 
  //    run
  //
  //     ├ subrun
  //     │  │  
  //     │  └ spill
  //
  //     └ calibration_period
  //
  //  This is done by specifying the hierarchy "paths".
 
  return d.driver(phlex::fixed_hierarchy{{"run", "subrun", "spill"},
                                         {"run", "calibration_period"}}, 
                  cells_to_process);
}

Behaviors:

  • The job data-cell cursor is always yielded by the framework first and then passed to the user's driver function (e.g., cells_to_process)
  • Only data-cell cursors supported by the specified phlex::fixed_hierarchy object may be yielded to the framework (validation happens whenever yield_child(...) is invoked)

Resolves #438.

@greenc-FNAL
Copy link
Copy Markdown
Contributor

Review the full CodeQL report for details.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@            Coverage Diff             @@
##             main     #443      +/-   ##
==========================================
+ Coverage   85.20%   86.56%   +1.36%     
==========================================
  Files         142      144       +2     
  Lines        3541     4951    +1410     
  Branches      610      616       +6     
==========================================
+ Hits         3017     4286    +1269     
- Misses        314      458     +144     
+ Partials      210      207       -3     
Flag Coverage Δ
unittests 86.56% <100.00%> (+1.36%) ⬆️

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

Files with missing lines Coverage Δ
phlex/app/load_module.cpp 90.00% <100.00%> (+3.95%) ⬆️
phlex/core/framework_graph.cpp 92.30% <100.00%> (+0.72%) ⬆️
phlex/core/framework_graph.hpp 100.00% <ø> (ø)
phlex/driver.hpp 100.00% <100.00%> (ø)
phlex/model/data_cell_index.cpp 90.52% <100.00%> (+0.05%) ⬆️
phlex/model/data_cell_index.hpp 0.00% <ø> (ø)
phlex/model/fixed_hierarchy.cpp 100.00% <100.00%> (ø)
phlex/model/fixed_hierarchy.hpp 100.00% <100.00%> (ø)
phlex/model/product_store.cpp 82.60% <100.00%> (-0.73%) ⬇️
plugins/generate_layers.cpp 100.00% <100.00%> (ø)
... and 2 more

... and 105 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 42ba142...7b9706a. Read the comment docs.

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

@beojan
Copy link
Copy Markdown
Contributor

beojan commented Mar 25, 2026

Can you make the hierarchy publicly accessible? We would need it at graph construction time to figure out the destination layer for transforms.

@knoepfel
Copy link
Copy Markdown
Member Author

Can you make the hierarchy publicly accessible? We would need it at graph construction time to figure out the destination layer for transforms.

Yes, I expect we'll need to make the hierarchy publicly accessible (i.e., provide some member function of phlex::fixed_hierarchy that returns a representation of the hierarchy). Right now, I'm not sure what the interface for the exposed hierarchy should look like. I don't believe the user needs it right now, but as we optimize the formation of the graph, exactly how the hierarchy should be exposed will become clearer.

@knoepfel knoepfel force-pushed the adjust-api branch 2 times, most recently from 9a87466 to e8989d2 Compare March 26, 2026 21:28
Copy link
Copy Markdown
Member

@marcpaterno marcpaterno left a comment

Choose a reason for hiding this comment

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

Saba and Marc have reviewed and have only one suggestion: rename data_cell to data_cell_cursor (because this is not the "data cell" described in the design doc).

@knoepfel knoepfel dismissed marcpaterno’s stale review March 27, 2026 19:47

Requested change implemented with 7b9706a.

@knoepfel knoepfel requested a review from marcpaterno March 27, 2026 19:48
@knoepfel knoepfel merged commit 56c9c5b into Framework-R-D:main Mar 27, 2026
35 of 36 checks passed
@knoepfel knoepfel deleted the adjust-api branch March 27, 2026 20:47
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.

Create driver interface for a fixed-hierarchy job

4 participants