Fix memory calculation for multiple sweep integration runs #1728
Conversation
Need to select the reflections for each experiment, not use all reflections for each image set. Fixes #1727.
There was a problem hiding this comment.
Change makes sense for the bug, thanks for fixing.
I thought it would be useful to start using the experiment identifier selection method, even though your selection wasn't wrong. I haven't verified this extra change on the data in question but I'm hoping it's trivial for you to do that?
|
@jbeilstenedmands testing |
i.e. more than one experiment belonging to an imageset, one should select all reflections belonging to that imageset for the memory calculation.
|
Having thought about this further, I think the proposed change set would not have correctly handled multi-lattice data, where there are several reflection tables associated with the imageset, but which get processed as one 'block' of data for integration (the memory requirement would have been underestimated). So I've updated to handle this too. |
Codecov Report
@@ Coverage Diff @@
## main #1728 +/- ##
==========================================
+ Coverage 66.65% 71.09% +4.44%
==========================================
Files 615 868 +253
Lines 68872 92399 +23527
Branches 9572 11799 +2227
==========================================
+ Hits 45905 65694 +19789
- Misses 21042 24616 +3574
- Partials 1925 2089 +164 |
|
Checking again following latest updates @ndevenish could be good to get this merged if looks OK to everyone |
|
I note there are now a lot of test failures in here 🤔 |
|
Oh, this is probably because a lot of the test data won't have any identifiers set. Let me think about this. |
Ah that makes some sense - indicates that we should probably update the test data but also a quick |
Co-authored-by: Markus Gerstel <2102431+Anthchirp@users.noreply.github.com>
Need to select the reflections for each experiment, not use all reflections for each image set. Fixes #1727.