-
Notifications
You must be signed in to change notification settings - Fork 69
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
Issue with parallelization for 5.2.2 #1117
Comments
One way we can get AttributeErrors when introducing a new default like FSSpecSource is by expecting it on all Sources but the new Source doesn't have it. That's not what happened here: Reading the code, it looks like uproot5/src/uproot/source/fsspec.py Lines 63 to 73 in 1cee960
Since I have not tested it, so let me know, @torresramiro350, if that's right. @lobis, is this correct? I'll add you as a reviewer to the PR so that you can weigh in there, but do you see anything else that might be a problem here? |
Introducing |
When @lobis approves it, I'll merge it. Thanks for testing! |
I also tried running the example above with uproot 5.2.2 and it seems to take considerably longer to read a ROOT file of ~ 1GB than in version 5.1.2. I'll try to generate a file with fake numbers that can somehow help to diagnose it further. I realize that will have to be on another issue. Thanks for the help! |
That should be a different issue. I've been hearing a few things about local file access being slower; maybe we should revert to MemoryMappedSource for local files and use fsspec only for remote files... |
I linked this issue to the PR after merging the PR. Closing the issue manually... |
In uproot 5.1.2, I was able to parallelize the reading of ROOT files with the following example below. However, with the new release
uproot 5.2.2
, I get this error:An example of the file can be retrieved here: https://data.hawc-observatory.org/datasets/geminga2017/geminga2017-download/maptree.root
Additionally, now with the new release I try to parallelize reading larger of +1GB ROOT files and what it used to take < 1 minute now runs for +3 minutes.
The text was updated successfully, but these errors were encountered: