root icon indicating copy to clipboard operation
root copied to clipboard

[ntuple] Reader: add option to enable metrics on construction

Open silverweed opened this issue 1 year ago • 2 comments

This Pull request:

adds the possibility of requesting metrics in RNTupleReadOptions. Since one normally has to call EnableMetrics() on the RNTupleReader to start collecting metrics, it is currently not possible to query the metrics gathered during construction of the reader itself. With this new option you can request metrics to be enabled at construction time.

As a somewhat arbitrary choice, an RNTupleReader created via Clone() will inherit this option from the one it is cloned from.

Checklist:

  • [x] tested changes locally
  • [ ] updated the docs (if necessary)

This PR fixes #

silverweed avatar Jun 28 '24 12:06 silverweed

Thanks, this is useful! In a similar vein, perhaps it would also be nice to have a HasMetricsEnabled method for the reader itself, especially if this option is inherited when cloning.

@enirolf I'm not sure that's necessary because one can already get that information with reader->GetMetrics().IsEnabled()

silverweed avatar Jun 28 '24 13:06 silverweed

Test Results

    13 files      13 suites   2d 22h 1m 45s :stopwatch:  2 651 tests  2 651 :white_check_mark: 0 :zzz: 0 :x: 32 645 runs  32 645 :white_check_mark: 0 :zzz: 0 :x:

Results for commit 12da709b.

github-actions[bot] avatar Jun 28 '24 15:06 github-actions[bot]

Thanks, this is useful! In a similar vein, perhaps it would also be nice to have a HasMetricsEnabled method for the reader itself, especially if this option is inherited when cloning.

@enirolf I'm not sure that's necessary because one can already get that information with reader->GetMetrics().IsEnabled()

Ah yes, you're right! I missed that method in the docs.

enirolf avatar Jul 01 '24 06:07 enirolf