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.

Sign up for GitHub

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

Jump to bottom

Erosion step in extended dispersion ignores global mask #14

Open
dimitrivlachos opened this issue Oct 24, 2024 · 1 comment
Open

Erosion step in extended dispersion ignores global mask #14

dimitrivlachos opened this issue Oct 24, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@dimitrivlachos
Copy link
Collaborator

dimitrivlachos commented Oct 24, 2024 •
edited
Loading

As elucidated in the erosion source code from #1, it would make sense that masked pixels should not be taken into account when doing erosion as their values cannot be trusted. However, DIALS itself does not appear to take the mask into account in this step.

See: https://github.com/dials/dials/blob/main/src/dials/algorithms/image/threshold/local.h

I have included the code that would do this, however it is currently commented out in order to match DIALS' results. I am not sure if this is intentional on DIALS' part or a bug, but it is something to keep in mind.
@dimitrivlachos dimitrivlachos added bug Something isn't working question Further information is requested labels Oct 24, 2024
@dimitrivlachos
Copy link
Collaborator Author

This has now been confirmed as a bug, see: dials/dials#2785

Whether we fix this or not is, as yet, undecided. Fixing it would be correct, but it would cause a deviation from DIALS.

@dimitrivlachos dimitrivlachos removed the question Further information is requested label Oct 28, 2024
dimitrivlachos added a commit that referenced this issue Nov 5, 2024
@dimitrivlachos
Implement extended dispersion spotfinding (#1)
db48d74 
Implement extended dispersion spotfinding

Implement a GPU-based version of the extended dispersion spotfinding
algorithm. This builds on regular dispersion by making two passes.
This allows for the detection of fainter spots by using the first pass
to detect candidate spots and exclude them from the background
calculation in the second pass.

Extended dispersion spotfinding is unavoidably slower than regular
dispersion by the fact that it requires two passes. However, the
performance gained through massively parallel processing on the GPU
should make this a viable option, when needed, even for fast feedback.

Create several CUDA kernels to perform the extended dispersion
spotfinding algorithm (`threshold.cu`, `erosion.cu`).

Refactor the dispersion kernel to share code with extended dispersion.

Move common code to `cuda_common.hpp`.

Create basic test script for extended dispersion spotfinding.

Add an `--algorithm` argument to `spotfinder.cc` along with the
necessary code to parse it, allowing for algorithm selection at runtime.

Add new files to the CMakeLists.txt file to include them in the build.

See also: #12, #13, #14
dimitrivlachos added a commit that referenced this issue Nov 15, 2024
@dimitrivlachos
Comment out unused parameters only necessary for #14
7647318
dimitrivlachos added a commit that referenced this issue Feb 4, 2025
@dimitrivlachos
Update fix for #14 to new interface
07dda74
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@dimitrivlachos