-
Notifications
You must be signed in to change notification settings - Fork 3
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
Split loading of monitors and detector data #82
Comments
Note that a first attempt at this was done when implementing the direct beam iterations. If I remember correctly, this was mostly for simplicity and clarity in the final task graph, but we did not rule out implementing this in the future. |
I have experiment with larger files today (up to about 1e9 in detector and monitor) and my conclusion was that we have to be more deliberate about dependencies in the graphs, such as avoiding keeping alive intermediates by using them in multiple places. We actually have to also sit down and have a look at final meta data assembly, e.g., as implemented in scipp/essreflectometry#27, as this potentially introduces long term "global" dependencies that result in keeping alive raw data longer than we want. @jl-wynen has already ran into some problem, I think, so we really have to figure out some guidelines/lessons. |
After some more thought, I believe we should also split loading "the rest" of the file: We need information such as chopper logs to be able to process either monitors or data, and there are probably also other examples such as determining slicing. So my suggestion is something like:
Furthermore, if we need to resort to chunking the processing of the events due to memory limitations, it would be advantageous to load and process the "rest" only once. Splitting this is probably also beneficial for reusing more code for the live-reduction, where we initially get the non-event data, and a series of updates containing events and log values. Make sure to see also scipp/beamlime#134. |
There are a couple reasons for this:
Should also be considered in relation to #83.
The text was updated successfully, but these errors were encountered: