Skip to content

Conversation

jackapet
Copy link

@jackapet jackapet commented Sep 25, 2025

This Pull request:

Add support for THnSparseD histograms in RDF

Changes or fixes:

  • Adding THnSparseDModel into HistoModels
  • Adding HistoNSparseD functions into RInterface
  • Remove deletion of copy constructor from THnSparse: THnSparse(const THnSparse&) = delete;

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

Fixes #19969

@ferdymercury

This comment was marked as outdated.

@jackapet jackapet changed the title Add support for THnSparseD histogram in RDataFrame Fixes #19969 Sep 25, 2025
@jackapet jackapet changed the title Fixes #19969 Fixes Add support for THnSparseD histogram in RDataFrame #19969 Sep 25, 2025
@jackapet jackapet changed the title Fixes Add support for THnSparseD histogram in RDataFrame #19969 Fixes #19969 Sep 25, 2025
@jackapet jackapet changed the title Fixes #19969 Add support for THnSparseD histogram in RDataFrame Sep 25, 2025
@jackapet
Copy link
Author

Thanks @ferdymercury ,

I will try to follow your instructions.

I noticed in my local tests that the THnSparseD and THnD too do not work with multithreading. Both seems to work fine with a single thread.

@ferdymercury
Copy link
Collaborator

THnSparseD and THnD too do not work with multithreading

See https://root-forum.cern.ch/t/filling-histograms-in-parallel/35460

Copy link
Collaborator

@ferdymercury ferdymercury left a comment

Choose a reason for hiding this comment

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

Thanks a lot! Feel free to click on the button "Ready to Review" so that core developers are notified.

@jackapet
Copy link
Author

THnSparseD and THnD too do not work with multithreading

See https://root-forum.cern.ch/t/filling-histograms-in-parallel/35460

I see, thank you for advice.

@jackapet
Copy link
Author

I implemented all required changes. I have few comments/questions:

  • I am not able to run tests locally because I do not have a necessary software installed. Is there some docker image which I can use for development/testing?
  • In documentation, I was not able to determine links to lines in the html file, e.g. [HistoNSparseD](https://root.cern/doc/master/classROOT_1_1RDF_1_1RInterface.html#a5f3e2f0a3d1c8e4f0e2f3e7f0e8c6b7a). The link was generated by copilot an I do not know if it is right.

@jackapet jackapet marked this pull request as ready for review September 25, 2025 13:49
Copy link
Collaborator

@ferdymercury ferdymercury left a comment

Choose a reason for hiding this comment

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

Concerning the link to the html file, leave it empty for the moment.
Thanks.

@ferdymercury
Copy link
Collaborator

One further comment: commit messages usually start with the involved component:

[math] fix sth
[RDF] improve Rdataframe...
[NFC] document better
(NFC is a signal for changes that are not relevant from the code point)

@jackapet
Copy link
Author

One further comment: commit messages usually start with the involved component:

[math] fix sth [RDF] improve Rdataframe... [NFC] document better (NFC is a signal for changes that are not relevant from the code point)

Ok, I added [RDF] or [NFC] to commits

@ferdymercury
Copy link
Collaborator

ferdymercury commented Sep 25, 2025

Thanks again!

In README/ReleaseNotes/v638/index.md
you could add:
- Add HistoNSparseD action that fills a sparse N-dimensional histogram.

  • You could add yourself as contributor on the list of that file.

nitpick wrt commit message: [hist] is usually small compared to [RDF] RDataFrame

@martamaja10
Copy link
Contributor

Hi @jackapet, thank you very much for this contribution! It looks good to me at the first glance, so I will let the CI run and once we see those results I will also give you a more detailed review. Thanks to @ferdymercury for giving the first reviews and hints as well!

@martamaja10 martamaja10 self-assigned this Sep 26, 2025
Copy link

github-actions bot commented Sep 26, 2025

Test Results

    22 files      22 suites   3d 22h 45m 36s ⏱️
 3 689 tests  3 687 ✅ 0 💤 2 ❌
79 225 runs  79 219 ✅ 0 💤 6 ❌

For more details on these failures, see this check.

Results for commit 7848f3c.

♻️ This comment has been updated with latest results.

@martamaja10
Copy link
Contributor

martamaja10 commented Sep 26, 2025

Hi @jackapet, so unfortunately there is an issue with your changes at the ROOT building stage. Have you tried building ROOT locally with all these changes? If you go through the logs of the CI workers, for example, you can see the following:

2025-09-26T10:28:40.3536950Z ##[error]/Users/sftnight/ROOT-CI/src/tree/dataframe/test/dataframe_histomodels.cxx:363:31: error: no matching constructor for initialization of '::THnSparseD' (aka 'THnSparseT<TArrayD>')
2025-09-26T10:28:40.3541740Z   363 |    auto h1e = d.HistoNSparseD(::THnSparseD("h1e", "h1e", 4, nbinse, edges), {"x0", "x1", "x2", "x3"});
2025-09-26T10:28:40.3643390Z       |                               ^            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-09-26T10:28:40.3746460Z /Users/sftnight/ROOT-CI/src/hist/hist/inc/THnSparse.h:72:4: note: candidate inherited constructor not viable: no known conversion from 'std::vector<std::vector<double>>' to 'const Double_t *' (aka 'const double *') for 5th argument
2025-09-26T10:28:40.3824260Z    72 |    THnSparse(const char* name, const char* title, Int_t dim,
2025-09-26T10:28:40.3855300Z       |    ^
2025-09-26T10:28:40.3878380Z    73 |              const Int_t* nbins, const Double_t* xmin = nullptr, const Double_t* xmax = nullptr,
2025-09-26T10:28:40.3879050Z       |                                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-09-26T10:28:40.3880090Z /Users/sftnight/ROOT-CI/src/hist/hist/inc/THnSparse.h:213:21: note: constructor from base class 'THnSparse' inherited here
2025-09-26T10:28:40.3880990Z   213 |    using THnSparse::THnSparse;
2025-09-26T10:28:40.3881340Z       |                     ^
2025-09-26T10:28:40.3882470Z /Users/sftnight/ROOT-CI/src/hist/hist/inc/THnSparse.h:75:4: note: candidate inherited constructor not viable: requires at most 4 arguments, but 5 were provided
2025-09-26T10:28:40.3883580Z    75 |    THnSparse(const char* name, const char* title,
2025-09-26T10:28:40.3884010Z       |    ^         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-09-26T10:28:40.3884430Z    76 |              const std::vector<TAxis>& axes,
2025-09-26T10:28:40.3884840Z       |              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-09-26T10:28:40.3885230Z    77 |              Int_t chunksize = 1024 * 16);
2025-09-26T10:28:40.3885610Z       |              ~~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-09-26T10:28:40.3886560Z /Users/sftnight/ROOT-CI/src/hist/hist/inc/THnSparse.h:213:21: note: constructor from base class 'THnSparse' inherited here
2025-09-26T10:28:40.3887390Z   213 |    using THnSparse::THnSparse;
2025-09-26T10:28:40.3887740Z       |                     ^
2025-09-26T10:28:40.3888920Z /Users/sftnight/ROOT-CI/src/hist/hist/inc/THnSparse.h:37:7: note: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 5 were provided
2025-09-26T10:28:40.3890820Z    37 | class THnSparse: public THnBase {
2025-09-26T10:28:40.3891190Z       |       ^~~~~~~~~
2025-09-26T10:28:40.3892050Z /Users/sftnight/ROOT-CI/src/hist/hist/inc/THnSparse.h:213:21: note: constructor from base class 'THnSparse' inherited here
2025-09-26T10:28:40.3892890Z   213 |    using THnSparse::THnSparse;
2025-09-26T10:28:40.3893260Z       |                     ^
2025-09-26T10:28:40.3894670Z /Users/sftnight/ROOT-CI/src/tree/dataframe/inc/ROOT/RDF/HistoModels.hxx:24:7: note: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 5 were provided
2025-09-26T10:28:40.3895860Z    24 | class THnSparseT;
2025-09-26T10:28:40.3896150Z       |       ^~~~~~~~~~
2025-09-26T10:28:40.3897450Z /Users/sftnight/ROOT-CI/src/tree/dataframe/inc/ROOT/RDF/HistoModels.hxx:24:7: note: candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 5 were provided
2025-09-26T10:28:40.3898520Z    24 | class THnSparseT;
2025-09-26T10:28:40.3898660Z       |       ^~~~~~~~~~
2025-09-26T10:28:40.3899270Z /Users/sftnight/ROOT-CI/src/tree/dataframe/inc/ROOT/RDF/HistoModels.hxx:24:7: note: candidate constructor (the implicit default constructor) not viable: requires 0 arguments, but 5 were provided
2025-09-26T10:28:40.3899780Z 1 error generated.
2025-09-26T10:28:40.3900280Z make[2]: *** [tree/dataframe/test/CMakeFiles/dataframe_histomodels.dir/dataframe_histomodels.cxx.o] Error 1
2025-09-26T10:28:40.3900690Z make[1]: *** [tree/dataframe/test/CMakeFiles/dataframe_histomodels.dir/all] Error 2

This will need to be fixed first.

@jackapet
Copy link
Author

jackapet commented Sep 26, 2025

Hello @martamaja10 ,

I am able to compile code without tests.

I was not able to run or compile tests because I am not able to install required system packages.

I am using windows with wsl2. ROOT compilation fails with RVec compilation:

/mnt/c/cern/root_dev/build/include/ROOT/RVec.hxx:516:49: error: use of undeclared identifier 'R__HARDWARE_INTERFERENCE_SIZE'
   static constexpr std::size_t cacheLineSize = R__HARDWARE_INTERFERENCE_SIZE;

I guess wsl2 does not have this variable defined. Is there some workaround for this?

When running on cluster, I am able to compile the ROOT but I can't install libraries. So, I was able to check rdataframe in my local script but not in required tests.

Could you provide instructions how to compile tests? Do you have a singularity/docker image for developers with all required libraries installed? I can access cvmfs if it helps. Otherwise, I will try to build my own image.

@ferdymercury
Copy link
Collaborator

when calling cmake, did you enable flag -Dtesting=ON -Droottest=ON ?

(Building is enough, no need to install)

@ferdymercury
Copy link
Collaborator

The failure is likely because there is something equivalent to

THn::THn(const char *name, const char *title, Int_t dim, const Int_t *nbins,

that needs to be implemented in THnSparse.h and .cxx

@jackapet
Copy link
Author

when calling cmake, did you enable flag -Dtesting=ON -Droottest=ON ?

(Building is enough, no need to install)

Ok, thank you. I am trying to recompile it. I made an apptainer image with settings below. I hope all libraries are now installed.

Bootstrap: docker
From: ubuntu:24.04

%post
    apt-get -y update
    apt-get -y install binutils cmake dpkg-dev g++ gcc libssl-dev git libx11-dev libxext-dev libxft-dev libxpm-dev python3 libtbb-dev libvdt-dev libgif-dev
    apt-get -y install gfortran libpcre3-dev libglu1-mesa-dev libglew-dev libftgl-dev libfftw3-dev libcfitsio-dev libgraphviz-dev libavahi-compat-libdnssd-dev libldap2-dev python3-dev python3-numpy libxml2-dev libkrb5-dev libgsl-dev qtwebengine5-dev nlohmann-json3-dev libmysqlclient-dev libgl2ps-dev liblzma-dev libxxhash-dev liblz4-dev libzstd-dev
    apt-get -y install libgtest-dev libgmock-dev
    apt-get -y install python3-pytest

@jackapet
Copy link
Author

jackapet commented Oct 2, 2025

I added the test into the dataframe_concurency.cxx.

The helgrind gives no error:

Apptainer> valgrind --tool=helgrind --suppressions=$ROOTSYS/etc/helgrind-root.supp root -x -q -l test_multithread.C+
==2912736== Helgrind, a thread error detector
==2912736== Copyright (C) 2007-2017, and GNU GPL'd, by OpenWorks LLP et al.
==2912736== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
==2912736== Command: root -x -q -l test_multithread.C+
==2912736==

Processing test_multithread.C+...
1e+06
bin 1 = 100000 expected: 100000
bin 2 = 100000 expected: 100000
bin 3 = 100000 expected: 100000
bin 4 = 100000 expected: 100000
bin 5 = 100000 expected: 100000
bin 6 = 100000 expected: 100000
bin 7 = 100000 expected: 100000
bin 8 = 100000 expected: 100000
bin 9 = 100000 expected: 100000
bin 10 = 100000 expected: 100000
==2912736==
==2912736== Use --history-level=approx or =none to gain increased speed, at
==2912736== the cost of reduced accuracy of conflicting-access information
==2912736== For lists of detected and suppressed errors, rerun with: -s
==2912736== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

@ferdymercury
Copy link
Collaborator

I guess helgrind with root.exe looks fine too?

@martamaja10 could you restart the CI ?

Thanks

@jackapet
Copy link
Author

jackapet commented Oct 2, 2025

Ah, sorry, I forgot on .exe. It does not look well with root.exe. It crashes. However, it does not seem to be related with THnSparseD.

Apptainer> valgrind --tool=helgrind --suppressions=$ROOTSYS/etc/helgrind-root.supp root.exe -x -q -l test_multithread.C+
==2926619== Helgrind, a thread error detector
==2926619== Copyright (C) 2007-2017, and GNU GPL'd, by OpenWorks LLP et al.
==2926619== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
==2926619== Command: root.exe -x -q -l test_multithread.C+
==2926619==

Processing test_multithread.C+...

valgrind: m_debuginfo/debuginfo.c:950 (truncate_DebugInfoMapping_overlaps): Assertion '!overlap' failed.

host stacktrace:
==2926619==    at 0x58027A0A: ??? (in /usr/libexec/valgrind/helgrind-amd64-linux)
==2926619==    by 0x58027B4F: ??? (in /usr/libexec/valgrind/helgrind-amd64-linux)
==2926619==    by 0x58027CE5: ??? (in /usr/libexec/valgrind/helgrind-amd64-linux)
==2926619==    by 0x5805A746: ??? (in /usr/libexec/valgrind/helgrind-amd64-linux)
==2926619==    by 0x5808A913: ??? (in /usr/libexec/valgrind/helgrind-amd64-linux)
==2926619==    by 0x58096D6B: ??? (in /usr/libexec/valgrind/helgrind-amd64-linux)
==2926619==    by 0x58086BA1: ??? (in /usr/libexec/valgrind/helgrind-amd64-linux)
==2926619==    by 0x580824B2: ??? (in /usr/libexec/valgrind/helgrind-amd64-linux)
==2926619==    by 0x580845C8: ??? (in /usr/libexec/valgrind/helgrind-amd64-linux)
==2926619==    by 0x580CFB07: ??? (in /usr/libexec/valgrind/helgrind-amd64-linux)

sched status:
  running_tid=1

Thread 1: status = VgTs_Runnable syscall 9 (lwpid 2926619)
==2926619==    at 0x4025D2C: __mmap64 (mmap64.c:58)
==2926619==    by 0x4025D2C: mmap (mmap64.c:46)
==2926619==    by 0x4007F78: _dl_map_segments (dl-map-segments.h:139)
==2926619==    by 0x4007F78: _dl_map_object_from_fd (dl-load.c:1258)
==2926619==    by 0x4009528: _dl_map_object (dl-load.c:2268)
==2926619==    by 0x400D8DB: dl_open_worker_begin (dl-open.c:578)
==2926619==    by 0x400151B: _dl_catch_exception (dl-catch.c:237)
==2926619==    by 0x400CD1F: dl_open_worker (dl-open.c:803)
==2926619==    by 0x400151B: _dl_catch_exception (dl-catch.c:237)
==2926619==    by 0x400D163: _dl_open (dl-open.c:905)
==2926619==    by 0x50AF1A3: dlopen_doit (dlopen.c:56)
==2926619==    by 0x400151B: _dl_catch_exception (dl-catch.c:237)
==2926619==    by 0x4001668: _dl_catch_error (dl-catch.c:256)
==2926619==    by 0x50AEC82: _dlerror_run (dlerror.c:138)
==2926619==    by 0x50AF25E: dlopen_implementation (dlopen.c:71)
==2926619==    by 0x50AF25E: dlopen@@GLIBC_2.34 (dlopen.c:81)
==2926619==    by 0x696AD57: cling::utils::platform::DLOpen(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*) (in /mnt/nfs17/jacka/root_dev/install/lib/libCling.so)
==2926619==    by 0x683FC44: cling::DynamicLibraryManager::loadLibrary(llvm::StringRef, bool, bool) (in /mnt/nfs17/jacka/root_dev/install/lib/libCling.so)
==2926619==    by 0x6744D1F: TCling::Load(char const*, bool) (in /mnt/nfs17/jacka/root_dev/install/lib/libCling.so)
==2926619==    by 0x4AB6C00: std::_Function_handler<bool (char const*), TSystem::CompileMacro(char const*, char const*, char const*, char const*, unsigned int)::{lambda(TString const&)#1}::operator()(TString const&) const::{lambda(char const*)#1}>::_M_invoke(std::_Any_data const&, char const*&&) (in /mnt/nfs17/jacka/root_dev/install/lib/libCore.so)
==2926619==    by 0x4ABF30C: TSystem::CompileMacro(char const*, char const*, char const*, char const*, unsigned int)::{lambda(char const*, std::function<bool (char const*)>)#1}::operator()(char const*, std::function<bool (char const*)>) const [clone .isra.0] (in /mnt/nfs17/jacka/root_dev/install/lib/libCore.so)
==2926619==    by 0x4AC3CA9: TSystem::CompileMacro(char const*, char const*, char const*, char const*, unsigned int) (in /mnt/nfs17/jacka/root_dev/install/lib/libCore.so)
==2926619==    by 0x674C3C9: TCling::ProcessLine(char const*, TInterpreter::EErrorCode*) (in /mnt/nfs17/jacka/root_dev/install/lib/libCling.so)
==2926619==    by 0x674CD73: TCling::ProcessLineSynch(char const*, TInterpreter::EErrorCode*) (in /mnt/nfs17/jacka/root_dev/install/lib/libCling.so)
==2926619==    by 0x4A3F056: TApplication::ExecuteFile(char const*, int*, bool) (in /mnt/nfs17/jacka/root_dev/install/lib/libCore.so)
==2926619==    by 0x487688F: TRint::ProcessLineNr(char const*, char const*, int*) (in /mnt/nfs17/jacka/root_dev/install/lib/libRint.so)
==2926619==    by 0x48784FC: TRint::Run(bool) (in /mnt/nfs17/jacka/root_dev/install/lib/libRint.so)
==2926619==    by 0x109302: main (in /mnt/nfs17/jacka/root_dev/install/bin/root.exe)
client stack range: [0x1FFEFF2000 0x1FFF000FFF] client SP: 0x1FFEFFA470
valgrind stack range: [0x1002E8E000 0x1002F8DFFF] top usage: 18232 of 1048576


Note: see also the FAQ in the source distribution.
It contains workarounds to several common problems.
In particular, if Valgrind aborted or crashed after
identifying problems in your program, there's a good chance
that fixing those problems will prevent Valgrind aborting or
crashing, especially if it happened in m_mallocfree.c.

If that doesn't help, please report this bug to: www.valgrind.org

In the bug report, send all the above text, the valgrind
version, and what OS and version you are using.  Thanks.

@ferdymercury
Copy link
Collaborator

Ah, sorry, I forgot on .exe. It does not look well with root.exe. It crashes. However, it does not seem to be related with THnSparseD.

Thanks. Let's the wait for a developer to start the CI.

Copy link
Collaborator

@ferdymercury ferdymercury left a comment

Choose a reason for hiding this comment

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

congrats @jackapet the CI is green

@jackapet
Copy link
Author

jackapet commented Oct 3, 2025

Hello, that's great that tests works now.

I see that code formatting checks fails. Is there a way how I can fix it?

@ferdymercury
Copy link
Collaborator

Is there a way how I can fix it?

If you click on the clang-format and go to the very bottom, they explain you can do this on the already checked out branch:

git rebase -i -x "git-clang-format master && git commit -a --allow-empty --fixup=HEAD" --strategy-option=theirs origin/master

However, sometimes clang-format does too aggressive changes and it introduces too much noise in the PR, so I'd say let's wait to see if a developer requests this formatting change or not.

@martamaja10
Copy link
Contributor

Hi @jackapet. Thank you for your changes and congrats on having the green CI. (P.S. sorry for my slow replies, I was sick last week).

Regarding clang-format - it will be needed.

My suggestion is to first clean up the commits a bit using interactive rebase - for example, squash the commits of RDF tests into one commit and improve the commit from "Adding tests" to "Adding tests for THnSparse action in RDataFrame". Similarly for the documentation. For the THnSparse implementation in RDataFrame you could just have one commit (so for example: Adding THnSparseDModel, Adding chunk size and Adding interfaces to HistoNSparseD could become a single commit). In the commit message you can add a small description of what exactly you did. For now, leave the [hist] commits as they are.

Once you have this sorted, you will have only a few commits. Then you can use command:
git clang-format HEAD~1 etc to only format what you changed in the given commit, which should be clean. If clang finds something to format, you'll need to add those files and then do git commit --amend.

@jackapet jackapet force-pushed the Add-THnSparseDModel branch from aa6b81b to 1e296c0 Compare October 7, 2025 07:42
@martamaja10
Copy link
Contributor

Hi @jackapet. Thanks for squashing the commits!

There are some issues with the CI - not because of your PR but we have just changed to LLVM 20 as of this morning. Could you rebase your branch to the current master and push your commits again?

Your local build after rebasing the master will also fail. What you can then do inside your build directory is:

 $ rm lib/*.pcm
 $ rm -rf interpreter/cling/tools/plugins/clad/

and then build again. If this doesn't work you might need to do a clean build from scratch.

Btw have you managed to do git clang-format?

@jackapet jackapet force-pushed the Add-THnSparseDModel branch from 1e296c0 to 7c054f2 Compare October 7, 2025 15:33
@jackapet
Copy link
Author

jackapet commented Oct 7, 2025

Hi @martamaja10 ,

I rebased the branch and I used clang to format the code. Could you rerun the CI again, please?

Petr

@jackapet jackapet force-pushed the Add-THnSparseDModel branch from 7c054f2 to 18dce68 Compare October 7, 2025 21:16
@jackapet
Copy link
Author

jackapet commented Oct 7, 2025

@martamaja10 , I fixed also the ruff formatting

Copy link
Member

@hageboeck hageboeck left a comment

Choose a reason for hiding this comment

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

Hello, thanks for the contribution!

I added some comments for your consideration.

@jackapet
Copy link
Author

Hello @hageboeck ,

thank you for your review and comments.

I think I implemented everything except of moving constructor of THnT to THn.cxx. I think this is not possible because it is a template class.

@jackapet jackapet force-pushed the Add-THnSparseDModel branch from 1432f62 to e43dedc Compare October 10, 2025 12:03
@hageboeck
Copy link
Member

I think I implemented everything except of moving constructor of THnT to THn.cxx. I think this is not possible because it is a template class.

That makes a lot of sense!

@jackapet jackapet force-pushed the Add-THnSparseDModel branch from 0cbd924 to 7294f6f Compare October 16, 2025 12:26
@jackapet
Copy link
Author

Hi @hageboeck , @martamaja10 ,

I fixed clang formatting in the last force push. May I ask to execute the pipeline again?

Petr

@ferdymercury
Copy link
Collaborator

Thanks. (Remaining test failures are not related to this PR)

Copy link
Contributor

@martamaja10 martamaja10 left a comment

Choose a reason for hiding this comment

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

Hi @jackapet - thank you very much for your PR, well done - many users will be very happy! Thank you for great communication and engagement - we really appreciate that :)

@martamaja10
Copy link
Contributor

martamaja10 commented Oct 17, 2025

@jackapet just one very last request before merging - could you please squash the commits (to end up just with one hist and one rdf commit)

Petr Jacka added 2 commits October 17, 2025 23:03
THnSparseD constructors consistent

Copy constructor is inmplemented for THnSparseD
Making THn and THnSparseD constructors consistent
Adding test of THn and THnSparseD constructors
THnSparseD model is added; Interfaces for HistoNSparseD are added
Adding tests for HistoNSparseD including concurrency
@jackapet jackapet force-pushed the Add-THnSparseDModel branch from 7294f6f to 7848f3c Compare October 17, 2025 21:05
@jackapet
Copy link
Author

@jackapet just one very last request before merging - could you please squash the commits (to end up just with one hist and one rdf commit)

Done

@jackapet
Copy link
Author

Hi all,

thank you very much for your help with the PR. It was very interesting to learn how the development works in root:)

@martamaja10 martamaja10 merged commit 6fe53e2 into root-project:master Oct 20, 2025
19 of 27 checks passed
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.

Add support for THnSparseD histogram in RDataFrame

5 participants