If crystal frame, use U for each crystal #1868
Conversation
Weird side effect of this is you never map the unindexed reflections. Need to iterate through and assign unindexed reflections a U matrix for mapping or similar. WIP for #1829
array_family/flex_ext.py
Outdated
| if "id" in self: | ||
| sel_expt = self["id"] == i | ||
| else: | ||
| sel_expt = self["imageset_id"] == i |
There was a problem hiding this comment.
Have you ran the indexing tests after making this change? I'm pretty sure this will result in some indexing failures as it explicitly relies on the existing behaviour - i.e. use the imageset_ids which are distinct, rather than the ids which will have been reset to -1 at the beginning of indexing to indicate "unindexed". (Yes, I know #1029 #1104.)
There was a problem hiding this comment.
Of course: this is precisely why I flagged this very early as a draft PR as I thought this will be über destructive - though that to me indicates that we have a different issue... but point well made.
There was a problem hiding this comment.
Unsurprisingly this breaks many things
Results (264.31s):
1022 passed
4 failed
- tests/algorithms/indexing/test_index.py:753 test_index_known_orientation
- tests/algorithms/indexing/test_assign_indices.py:247 test_local_multiple_rotations
- tests/command_line/test_export_best.py:4 test_export_best
- tests/command_line/test_search_beam_position.py:170 test_search_small_molecule
157 skipped
array_family/flex_ext.py
Outdated
| if "id" in self: | ||
| sel_expt = self["id"] == i | ||
| else: | ||
| sel_expt = self["imageset_id"] == i |
There was a problem hiding this comment.
Perhaps the following would solve your problem in a less destructive way?
| if "id" in self: | |
| sel_expt = self["id"] == i | |
| else: | |
| sel_expt = self["imageset_id"] == i | |
| if not crystal_frame and "imageset_id" in self: | |
| sel_expt = self["imageset_id"] == i | |
| else: | |
| sel_expt = self["id"] == i |
There was a problem hiding this comment.
Am doing a little bit of hunting around - I have a suspicion that there are better ways to fix this problem, as there are other derived questions from this (like: is this part of the reason why re-running indexing on multi-lattice data does not re-index all lattiecs)
There was a problem hiding this comment.
BTW I recognise that this would fix the behaviour but I worry that it is not fixing the underlying problem.
There was a problem hiding this comment.
This change also broke indexing with > 1 lattice...
|
Looking at the code @rjgildea if there are multiple experiments with the same |
As suggested by @rjgildea but with added comments to explain to me-in-two-years
Codecov Report
@@ Coverage Diff @@
## main #1868 +/- ##
=======================================
Coverage 67.15% 67.15%
=======================================
Files 617 617
Lines 69532 69532
Branches 9672 9672
=======================================
+ Hits 46695 46697 +2
+ Misses 20894 20892 -2
Partials 1943 1943 |
If crystal_frame: iterate through experiments not image sets, performing the mapping for each unique experiment to put data into crystal frame. By definition then does not show _unindexed_ reflections as these have no crystal frame. Fixes #1829 Co-authored-by: Nicholas Devenish <ndevenish@gmail.com> Co-authored-by: Richard Gildea <rjgildea@users.noreply.github.com>
Bugfixes -------- - ``dials.reciprocal_lattice_viewer``: In cases with multiple lattices, "Crystal Frame" now aligns all crystal frames, rather than just the first. Unindexed reflections are no longer shown in this mode. (#1868)
Weird side effect of this is you never map the unindexed reflections. Need to
iterate through and assign unindexed reflections a U matrix for mapping or
similar.
WIP for #1829