-
Notifications
You must be signed in to change notification settings - Fork 89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add .root to .sqlite converter #514
Conversation
Update essnusb but keep dataset changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @kaareendrup!
Let me start by apologizing for the wait; it has been quite hectic since the workshop! Second; thanks for sharing this code.
The DataConverter
was written to extract i3 -> [parquet, SQLite]
so there is indeed some i3 specific code in there. However, much of it is completely independent of IceCube software, and I think we can come up with a nice way to structure this. I will give this a think and return with some comments on how next week. In the mean time:
You mention that the computational bottleneck is in the submission of dataframes to SQLite format and that multiprocessing therefore isn't relevant for root files. If you have everything in one root file, I can see how this can be the case. But do you not have many root files that needs to be converted? The multiprocessing is currently implemented such that many files are converted in parallel instead of sequentially, which should give you a linear speedup in n_workers
.
Could you please install pre-commit
hooks and update the code such that the checks pass? More info here
Hi @RasmusOrsoe ! No problem, we are all busy, and despite my good intentions, it will likely also take a while before I implement your suggestions. I shall definitely take a look at the pre-commit hooks. I was only unsure about whether or not it was worthwhile at this point. Regarding multiprocessing, my experience has been that the overhead when combining the temporary .sqlite-files is greater than/comparable to that of processing the .root-files, so no speed was gained when processing the .root-files in parallel. But I of course don't know if this will always be the case, so I'm fine with implementing multiprocessing just in case. |
@kaareendrup I have given this some more thought, and come up with an idea draft. Below is pseudo-code that lays out the refactor of Starting with
This puts core functionality that is independent of both input and output file formats, such as batch patterns, boiler plate processing and parallelism into one flexible class. Implementing an i3-specific or root-specific version just requires overwriting a couple of abstract methods. Here's the i3-specific version:
Similarly, we can move input file independent functionality from
We could also go further and make our current back-end specific implementations of |
@kaareendrup we've recently updated the data conversion code, which would make this very easy. Is this still relevant of ESS? |
@kaareendrup closing this due to inactivity. Feel free to open this again if you want to add this contribution in the future |
.root to .sqlite conversion
This is an initial attempt at implementing a converter class (and subsequent extractor-classes) for .root to .sqlite conversion. The implementation follows the structure of the current .sqlite converter, with some exceptions and caveats as described in the following.
The following are notable exceptions to the I3 to .sqlite converter:
No multiprocessing
Since the computational bottleneck in case of .root files is submitting dataframes to the .sqlite-databases, there is no multiprocessing. Batchwise conversion is implemented to avoid having the entire database in memory at once.
No pattern batching
This should be relatively easy to implement, but I didn't understand it, so I chose to not bother
No inheritance from current classes
Perhaps the most important one. Since the current base classes have some I3 specific funtionality (like attaching gcd-files to lists of I3 files), the new classes don't inherit from them. This could perhaps be basis for a restructuring of the converter and extractor classes, if the goal is to provide more general functionality for different experiments.
There are probably many other things that I have missed or misunderstood, but I thought it would be good to join the forces of those who have worked hard on the implementation of the current classes with those who have an eye for root functionality.
Use
The classes use the uproot package and awkward pandas (which would maybe need to be added to the requirements). A simple conversion example (for ESSnuSB) could look like:
I'm unsure about how well this will generalize to different tree structures, but the combination of different main_keys and branch_keys should give a lot of flexibility.