Skip to content
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

omit overlapping shoeboxes because we can't handle them yet #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

omit overlapping shoeboxes because we can't handle them yet #4

wants to merge 1 commit into from

Conversation

dwpaley
Copy link
Contributor

@dwpaley dwpaley commented May 10, 2021

No description provided.

@nksauter
Copy link
Owner

As I understand it, this code removes dials spotfinder spots when the combined spot+three-pixel border overlaps with another. I would rather not throw out spotfinder spots, for reasons already discussed--it seems like an arbitrary decision, not easily explained in a paper. My preference would be to modify the code that creates the three-pixel border, just to assure that two refls do not claim the same border pixels. Perhaps it could be first come, first served. You could use a dictionary to keep a list of all total pixels claimed by previous spots in the list. I would rather go with this alternate idea.

Copy link
Owner

@nksauter nksauter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternate approach suggested above.

@dermen
Copy link
Collaborator

dermen commented May 10, 2021

I do this here - the first ROI in the list of ROIS gets preference and can claim all of its pixels.. then if a subsequent ROI tries to claim any already claimed pixels, the ROI is shrunk by a pixel in each direction until it is no longer asking for claimed pixels .

https://github.com/cctbx/cctbx_project/blob/6ec995a7c3565e2b077b6e5d043f6cced8952699/simtbx/diffBragg/utils.py#L657

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants